Skip to content

refactor: deepen three shallow seams (TurnPump, config writes, branch queries) - #27

Open
perezdap wants to merge 3 commits into
mainfrom
refactor/deepen-modules
Open

refactor: deepen three shallow seams (TurnPump, config writes, branch queries)#27
perezdap wants to merge 3 commits into
mainfrom
refactor/deepen-modules

Conversation

@perezdap

Copy link
Copy Markdown
Owner

What

Three deepening refactors from an architecture review of the hot spots (Program.cs, Chat/REPL cluster, providers + config). One commit each, individually green.

  1. ActiveBranch query module (ac34e46) — the nine hand-rolled parentId walks across the Chat cluster (last-of-kind ×3, count ×2, usage/char/token scans ×3, flatten ×2) now live behind one module loaded once from ISessionManager.GetActiveBranch(). SessionManager untouched. Also removes an accidental O(n²) walk in ChatSessionBootstrap.
  2. One config mutation seam (d4bb530) — ProviderConfigurator gains SetDefaultProviderAsync (with default-model repair), SetDefaultModelAsync, and a pure RepairDefaultModel shared by providers use and /provider. ConfigFileUpdater — the unvalidated JSON splice path — is deleted; every default-setting write now crosses the validator.
  3. TurnPump (a5aecde) — the Turn event-consumption loop was hand-written three times (REPL text, one-shot JSON, RPC) and had drifted. One module now owns stream order, the terminal-event protocol (FailedCompleted("partial"|"completed"), previously undocumented), artifact append on success and failure, usage extraction, and the steering-promotion policy. Front-ends plug in as presentation adapters; rendering behavior is byte-identical and RPC keeps its steering semantics.

Behavior deltas (intentional, reviewed)

  • providers use / models use now write canonical id casing and config.json is rewritten compact with defaults materialized — lossless for every key WinHarness owns, and now validated on save.
  • RPC steering semantics unchanged (no invented follow-ups); the previously-drifted policy now has one named, documented home.

Verification

  • dotnet build WinHarness.sln -c Release: 0 warnings, 0 errors (TreatWarningsAsErrors gate).
  • dotnet test WinHarness.sln -c Release --no-build: 386 passed, 0 failed, 0 skipped (baseline 356; +30 new tests: ActiveBranch 11, configurator/repair 12, TurnPump 7).
  • Middle commit d4bb530 built + tested standalone in a temp worktree (379 green) so every commit is bisect-safe.

Follow-ups (not in this PR)

  • RpcHost.HandleSetModel mutates provider/model without appending a ModelChangeSessionEntry — pre-existing drift outside the turn loop.
  • Three further candidates from the same review (slash-command registry, capability-surface consume-vs-cut, OAuth login orchestration) deferred pending design decisions.

perezdap added 3 commits July 17, 2026 13:52
…nch walks

One query module over the active branch (last-of-kind, count, usage
totals, flatten-to-Conversation) loaded once via ISessionManager.
Callers in Chat/ stop re-walking parentId links; semantics preserved.
…rConfigurator

Widen the configurator (SetDefaultProviderAsync with repair,
SetDefaultModelAsync, pure RepairDefaultModel shared with /provider)
so every default-setting write crosses the validated seam. Delete
ConfigFileUpdater, the unvalidated JSON splice path.
One module consumes the AgentEvent stream for all front-ends: it owns
stream order, the terminal-event protocol (Failed, then Completed with
"partial" or "completed"), artifact append on success and failure,
usage extraction, and the steering-promotion policy. The REPL, JSON
event stream, and RPC host plug in as presentation adapters; rendering
behavior is unchanged and RPC keeps its steering semantics.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant