fix(pi): review fixes — typed effort, live model fallback, menu-declared variants, shared session machinery#907
Merged
gewenyu99 merged 8 commits intoJul 15, 2026
Conversation
…red variants, shared session machinery Follow-ups from the review of #853: - effort is a ThinkingLevel end to end, validated at the parse boundary: a typo'd effort_pi in remote frontmatter is logged and dropped (the model-table default applies) instead of riding into a session through a cast. promptModelFor/taskModelSpec take the Harness enum, not a string. - taskModelSpec no longer bakes in DEFAULT_TASK_MODEL, which made the call sites' `?? pick.model` dead code: a pi task whose prompt names no model_pi now degrades to the switchboard's pi model, not sonnet-over-pi. Each harness falls back within its own column. - terra reverts to low in MODEL_CAPABILITIES: the table tunes the linear run only, and orchestrator agents already carry their own effort via prompt frontmatter (which overrides the table), so the medium row was a side effect on linear runs that the orchestrator never needed. - FRAMEWORK_VARIANT_ALIASES and the startsWith heuristic are gone: skill-menu.json entries now declare `group`/`framework`/`default` (context-mill PR on experiment/orchestrator), so variant resolution is an exact lookup, misses preflight before any agent runs (log + a new `orchestrator skill variant missing` capture), and the variant test pins the real menu contract instead of a fixture that mirrors the code. - new pi/shared.ts holds the session machinery both entry points use — gateway/registry wiring, hermetic loader, coding-tool factories, the session event watcher, error classification, usage capture — so run() and runPiTask() are configurations of one implementation and the 429 sniff / analytics shape can't drift. The linear run gains the parseable usage log line; the per-task line drops its modelId-as-task fallback. - loadAgentRegistry passes the menu entry's flow into parseAgentPrompt as the frontmatter fallback, so a prompt the menu placed in a flow is never silently dropped by the registry filter. Generated-By: PostHog Code Task-Id: fafc230d-6f14-4e4d-9462-0e7f18a1eec1
🧙 Wizard CIRun the Wizard CI and test your changes against wizard-workbench example apps by replying with a GitHub comment using one of the following commands: Test all apps:
Test all apps in a directory:
Test an individual app:
Show more apps
Results will be posted here when complete. |
Generated-By: PostHog Code Task-Id: fafc230d-6f14-4e4d-9462-0e7f18a1eec1
The medium row is terra's default for any run that doesn't name an effort itself; the "linear isolation" concern was hypothetical (linear doesn't run terra) and the revert only degraded the real default. models.ts and switchboard.test.ts terra lines are back to the base branch verbatim. Generated-By: PostHog Code Task-Id: fafc230d-6f14-4e4d-9462-0e7f18a1eec1
The switchboard is the single decision point; a shared layer between it and the runners is a place decisions could creep into. index.ts, task.ts, and subagent.ts return to the base branch verbatim; the typed effort in gateway.ts/types.ts stays. Generated-By: PostHog Code Task-Id: fafc230d-6f14-4e4d-9462-0e7f18a1eec1
- taskModelSpec takes the full HarnessPick and absorbs the switchboard fallback, so the enqueue-override -> frontmatter -> pick precedence lives in one place instead of being restated at each call site. - onTransition builds its analytics base lazily — enqueue/requeue never read it, and resolveHarness logs per call. - resolveReferenceSkillId (one-line wrapper, single call site) inlined. Generated-By: PostHog Code Task-Id: fafc230d-6f14-4e4d-9462-0e7f18a1eec1
Drops the /simplify reshaping (HarnessPick signature, lazy analytics base, wrapper inlining) and the Harness-enum param typing — taskModelSpec keeps its base shape minus the baked-in default, and call sites keep their existing pick fallback. Generated-By: PostHog Code Task-Id: fafc230d-6f14-4e4d-9462-0e7f18a1eec1
…s dropped Generated-By: PostHog Code Task-Id: fafc230d-6f14-4e4d-9462-0e7f18a1eec1
Preflight already logs + captures a missing skill variant on every build. Now, in dev and CI builds, it also throws so the gap fails a test run loudly instead of passing as a silent skill-less (zero-diff) integration. The throw is gated behind !IS_PRODUCTION_BUILD, which tsdown inlines to a literal, so it is tree-shaken out of the published bundle — verified: the throw string is absent from `pnpm build` output and present in `pnpm build:ci` output, while the log + analytics capture survive in both. Generated-By: PostHog Code Task-Id: fafc230d-6f14-4e4d-9462-0e7f18a1eec1
gewenyu99
marked this pull request as ready for review
July 15, 2026 17:42
gewenyu99
merged commit Jul 15, 2026
de41ed5
into
experiment/orchestrator-pi-runtask
8 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Review fixes for #853, targeting its branch. Pairs with PostHog/context-mill#239, which emits the
group/framework/defaultmenu fields the resolver here consumes — the two land on their experiment branches together.Changes
effort_pi/effort_sdkare validated againstThinkingLevelinparseAgentPrompt— a typo in remote frontmatter is logged and dropped (table default applies) instead of riding into a session through theas ThinkingLevelcast (removed fromgateway.ts). The value stays typed throughTaskRunInputs.taskModelSpecno longer bakes inDEFAULT_TASK_MODEL, so the call sites' existing?? pick.modelfallback is live: a pi task whose prompt names nomodel_pidegrades to the switchboard's pi model, not sonnet-over-pi.FRAMEWORK_VARIANT_ALIASESand thestartsWithguessing are gone;resolveSkillVariantIdis an exact lookup on the menu'sgroup/framework/defaultfields (context-mill owns the vocabulary). Task skills preflight after the registry loads — a miss logs and firesorchestrator skill variant missingbefore any agent runs. The test fixture is pinned from the real builtskill-menu.json.loadAgentRegistrypasses the menu entry'sflowintoparseAgentPromptso a prompt with missing frontmatter flow isn't silently dropped (context-mill's build is now strict about the key, too).Testing
pnpm build && pnpm test— 99 files, 1344 tests green; lint clean.Created with PostHog Code