Private
Public Access
0
0
Commit Graph

8 Commits

Author SHA1 Message Date
ed 4ab7c732b5 refactor(src): Phase 12.6.2-12.6.13 - migrate 16 small files to Result[T]
Migrated 27 silent-fallback/UNCLEAR sites across 16 sub-track 2 files:
- src/diff_viewer.py (1: apply_patch_to_file)
- src/presets.py (2: load_all global/project preset parsing)
- src/theme_models.py (2: load_themes_from_dir, load_themes_from_toml)
- src/summarize.py (3: _summarise_python, summarise_file x2)
- src/command_palette.py (1: _execute)
- src/markdown_helper.py (2: _on_open_link, render table fallback)
- src/commands.py (2: generate_md_only, save_all)
- src/conductor_tech_lead.py (1: topological_sort)
- src/orchestrator_pm.py (1: generate_tracks JSON parse)
- src/project_manager.py (1: get_git_commit)
- src/session_logger.py (1: log_tool_call write_ps1)
- src/shell_runner.py (1: run_powershell error)
- src/multi_agent_conductor.py (4: run, run_worker_lifecycle x3)
- src/aggregate.py (4: is_absolute_with_drive, build_file_items x2, build_tier3_context)
- src/warmup.py (1: _warmup_one indirect Result)
- src/models.py (2: from_dict discussion.ts, load_mcp_config)

Each migration follows the data-oriented convention:
- try/except body constructs a Result dataclass with ErrorInfo
- Pattern matches Heuristic A (Result-returning recovery)
- The Result carries the error info for telemetry/debugging

Added Result imports to: diff_viewer, presets, theme_models, summarize,
command_palette, markdown_helper, commands, conductor_tech_lead,
project_manager, shell_runner, multi_agent_conductor, models.

Audit post-fix: 0 violations, 0 UNCLEAR in sub-track 2 scope.
The remaining 152 violations are in sub-track 3 (mcp_client, app_controller)
+ sub-track 4 (gui_2) + sub-track 5 (ai_client, rag_engine baseline).
2026-06-18 10:21:24 -04:00
ed 4c42bd0545 refactor(src): warmup.py Phase 11.3.1 - FULL Result[T] migration (5 sites)
Phase 11.3.1 (REJECT Phase 10's sliming). Per the user's explicit
direction: every try/except site that can fail MUST return Result[T].
No 'user callback' excuse; the user callbacks in WarmupManager are
Callable[[dict], None] and stay as-is. The MANAGER's INTERNAL methods
return Result[T].

Changes:
- on_complete() returns Result[bool]; fires callback via _fire_callback
  helper that captures user-callback exceptions as ErrorInfo.
- _record_success() returns Result[bool]; aggregates per-callback errors.
- _record_failure() returns Result[bool]; same pattern.
- _log_canary() returns Result[None]; uses _log_stderr helper.
- _log_summary() returns Result[None]; uses _log_stderr helper.
- _warmup_one() (io_pool callback) returns Result[bool]; delegates to
  _record_success/_record_failure.
- _log_stderr() (new helper) returns Result[None]; captures OSError.
- _fire_callback() (new helper) returns Result[bool]; captures
  user-callback exceptions.

Audit post-migration:
- L319 (_log_stderr) = INTERNAL_COMPLIANT (Heuristic A) ✓
- L337 (_fire_callback) = INTERNAL_COMPLIANT (Heuristic A) ✓
- L185 (_warmup_one) = INTERNAL_BROAD_CATCH (known limitation:
  indirect return via 'return self._record_failure(...)' is not
  detected by Heuristic A which matches 'return Result(...)' directly)
- L96 (submit raise RuntimeError) = INTERNAL_RETHROW (programmer
  error, not a runtime failure; acceptable)

Tests: 16/16 pass (test_api_hooks_warmup.py, test_gui_warmup_indicator.py).

Per conductor/tracks/result_migration_small_files_20260617/plan.md
section 11.3.1.
2026-06-18 00:06:11 -04:00
ed 00eaa460fd refactor(src): Phase 10.2 batch 6 - hot_reloader + warmup + startup_profiler
hot_reloader.py (1 site - module reload with broad except):
- reload() returns Result[bool] now. The migration catches the
  broad Exception, captures it as ErrorInfo with the traceback in
  last_error, and returns Result(data=False, errors=[...]).
- reload_all() returns Result[bool]; aggregates per-module errors.
- The class still tracks last_error and is_error_state for
  backwards-compat with any caller reading the class attributes.

warmup.py (5 sites):
- L139 (on_complete callback fire): was except ...: pass.
  Now logs to sys.stderr with the exception.
- L215 (_record_success callback fire): same.
- L249 (_record_failure callback fire): same.
- L276 (_log_canary stderr.write): was except OSError: pass.
  Now logs the OSError itself.
- L300 (_log_summary stderr.write): same.

startup_profiler.py (1 site - context manager):
- phase() is a context manager (yields); can't return Result.
  The except inside the finally block now logs the OSError.

Tests updated for hot_reloader to check result.ok and result.data.

Tests verified:
- tests/test_hot_reloader.py (9 tests) PASS
- tests/test_hot_reload_integration.py (13 tests) PASS
- tests/test_warmup.py (10 tests) PASS
- tests/test_warmup_canaries.py (18 tests) PASS
2026-06-17 22:42:10 -04:00
ed c329c86931 refactor(src): narrow exception types in Phase 8 MEDIUM files (10 sites across 2 files)
Migrates the MEDIUM files (session_logger, warmup) by narrowing
the exception types from broad 'except Exception' to specific
stdlib exceptions.

session_logger.py (8 sites):
1. L99 - registry.register_session with print
   except Exception -> except (OSError, KeyError, AttributeError, TypeError)
2. L131 - registry.update_auto_whitelist_status with print
   except Exception -> except (OSError, KeyError, AttributeError, TypeError)
3. L147 - log_api_hook write/flush
   except Exception -> except (OSError, UnicodeEncodeError, ValueError)
4. L160 - log_comms json.dump
   except Exception -> except (OSError, TypeError, ValueError)
5. L188 - log_tool_call script file write
   except Exception -> except (OSError, UnicodeEncodeError)
6. L201 - log_tool_call write/flush
   except Exception -> except (OSError, UnicodeEncodeError, ValueError)
7. L226 - log_tool_output write_text
   except Exception -> except (OSError, UnicodeEncodeError)
8. L245 - log_cli_call write/flush
   except Exception -> except (OSError, TypeError, ValueError)

warmup.py (2 sites):
1. L276 - _log_canary sys.stderr.write
   except Exception -> except OSError
2. L300 - _log_summary sys.stderr.write
   except Exception -> except OSError

Decisions:
- warmup.py L85: raise RuntimeError (validation raise) - keep as-is per spec
- warmup.py L139, L215, L249: callback fires with except Exception - keep
  (user callbacks can throw anything; broad catch is correct)
- warmup.py L175: _warmup_one with except BaseException - keep
  (intentional broad catch for module import failures)

Tests verified:
- tests/test_session_logging.py (1 test) PASS
- tests/test_session_logger_reset.py (1 test) PASS
- tests/test_session_logger_optimization.py (4 tests) PASS
- tests/test_logging_e2e.py (1 test) PASS
- tests/test_warmup.py (10 tests) PASS
- tests/test_warmup_canaries.py (18 tests) PASS
2026-06-17 19:22:56 -04:00
ed 8d58d7fc46 fix(warmup): defer _done_event.set() until after callbacks fire
WarmupManager._record_success and _record_failure used to set
self._done_event.set() inside the with self._lock: block, BEFORE
calling the user-registered on_complete callbacks. This created
a race: a test thread calling mgr.wait() could observe
mgr.is_done() == True and proceed before the worker thread had
finished firing the callbacks. The mgr.on_complete caller would
then assert on state that the callback was supposed to mutate
(e.g. test_warmup_on_complete_callback_fires' `received` list).

Fix: move self._done_event.set() to AFTER the for cb in callbacks:
loop in both _record_success and _record_failure. The done event
is now set last, so wait() cannot return until all callbacks
have completed (or raised, which is swallowed by the try/except).

ALSO fix the previously-corrupted state of warmup.py (the result
of a misused set_file_slice edit that left orphaned code with no
def line for _record_failure). _record_failure is now a proper
class method with the def line restored.

ALSO fix tests/test_warmup.py:
  - test_warmup_on_complete_callback_fires: the test body was
    missing the pool/mgr setup. Added the missing lines.
  - test_warmup_done_event_set_after_all_complete: removed the
    racy `assert not mgr.is_done()` assertion that fires
    immediately after submit. On a fast machine, os/sys warmup
    completes in microseconds, so is_done() is already True
    by the time the assertion runs. The remaining assertion
    (`assert mgr.is_done()` after wait) still tests the
    semantic that the done event is set after completion.
  - Removed both `@pytest.mark.skip` markers; the underlying
    issues are now fixed in production code AND the tests.

Verified: 10/10 tests in tests/test_warmup.py pass (previously
2 skipped, 2 failed).
2026-06-07 16:02:30 -04:00
ed 152605f5dc feat(warmup): log canaries to stderr by default (with main-thread violation warning)
Per module: prints a one-line summary to stderr when the import
completes or fails:
  [warmup 1] google.genai on controller-io_0 (id=18636): 1218.6ms
  [warmup 2] anthropic on controller-io_1 (id=5500): 1148.3ms
  [warmup 3] openai on controller-io_2 (id=34376): 1144.2ms
  ...

When the entire warmup completes, prints an aggregate:
  [warmup done] 9 modules: 9 completed (sum of per-module elapsed: 3591.7ms)

If ANY canary ran on the main thread (main-thread-purity violation),
the per-module line is tagged with [MAIN-THREAD] AND a final WARNING
is printed:
  [warmup WARNING] N module(s) loaded on the MAIN THREAD: google.genai

Default is log_to_stderr=True so production runs get the observability
for free. Tests opt out via WarmupManager(pool, log_to_stderr=False)
in the _build_warmup helper.

5 new tests (4 stderr logging + 1 quiet). All 13 canary tests pass.

Use case: 'did my heavy import run on the GUI thread when it shouldnt
have?' is now answered by grepping stderr for [warmup ...] [MAIN-THREAD]
lines. No hook server required.
2026-06-06 22:15:24 -04:00
ed 208aa664db feat(warmup): per-module canary records (thread + timing observability)
Adds a canary record for each module submitted to the warmup, tracking:
canary_id, module, thread_name, thread_id, submit_ts, start_ts,
end_ts, elapsed_ms, status, error.

Surface:
- WarmupManager.canaries() returns list[dict] (defensive copy)
- AppController.warmup_canaries() returns list[dict] (delegation)
- GET /api/warmup_canaries Hook API endpoint
- ApiHookClient.get_warmup_canaries() returns list[dict]

Example: the warmup of google.genai records a 1187ms canary on
thread controller-io_0 with thread_id 50420, canary_id 1.

11 new tests (8 unit in test_warmup_canaries + 3 in test_api_hooks_warmup).
All pass; live_gui smoke test confirms endpoint returns real data.
2026-06-06 22:02:35 -04:00
ed 1354679e33 feat(io_pool, warmup): add shared 4-thread pool + WarmupManager
Phase 2 Tasks T2.1-T2.4 of the startup_speedup_20260606 track.

NEW: src/io_pool.py
  make_io_pool() factory: 4-worker ThreadPoolExecutor with
  thread_name_prefix='controller-io'. The sanctioned way for any
  background work. Replaces ad-hoc threading.Thread() calls per
  the 'no new threads' rule.

NEW: src/warmup.py
  WarmupManager: manages a list of modules to import on the shared
  pool. Public API:
    .submit(modules)        - start warmup (call once)
    .status()               - {pending, completed, failed}
    .is_done()              - bool
    .wait(timeout)          - block until done
    .on_complete(callback)  - register completion callback
    .reset()                - clear state
  Thread-safe (lock-guarded). 10 tests cover all paths.

NEW: tests/test_io_pool.py (4 tests):
  - ThreadPoolExecutor returned
  - 4 workers
  - Threads named 'controller-io-*'
  - Jobs run in parallel (barrier test)

NEW: tests/test_warmup.py (10 tests):
  - One job per module submitted
  - Initial pending list correct
  - Failed imports tracked
  - Done event set after all complete
  - wait() blocks until done
  - on_complete callback fires (and immediately if already done)
  - Modules actually end up in sys.modules
  - reset() clears state
  - Jobs run concurrently (not serially)

All 14 tests pass. AppController integration is the next commit.
2026-06-06 14:47:02 -04:00