feat(factory): wire per-role reasoning_effort from LlmConfig through spawn pipeline#4
Merged
Conversation
…ugh PtyConfig::claude test-first gate: new tests verify that: - custom effort value (e.g. "low") is used when provided - supervisor defaults to "high" when effort=None - worker defaults to "medium" when effort=None These tests require the new `effort: Option<&str>` parameter on PtyConfig::claude which does not exist yet — compilation failure is the expected failing state. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… factory spawn Reads `llm.supervisor.reasoning_effort` / `llm.worker.reasoning_effort` from `.cas/config.yaml` and threads the value all the way to the `--effort` flag passed to Claude CLI at spawn time. Changes: - cas-pty: add `effort: Option<&str>` to PtyConfig::claude and PtyConfig::codex; replace hardcoded high/medium with role-based fallback that respects the config value when set (backward-compatible: None → "high" for supervisor, "medium" for worker) - cas-mux: add `effort` param to Pane::worker() and Pane::supervisor(); add supervisor_effort/worker_effort to MuxConfig and Mux, wire through factory() and add_worker() - cas-factory: add supervisor_effort/worker_effort fields to FactoryConfig; wire into FactoryCore::new() (set_worker_effort) and spawn_supervisor() - cas-cli: populate the new FactoryConfig fields via llm.reasoning_effort_for_role() in both factory entry points (mod.rs + daemon.rs) and propagate through the two MuxConfig construction sites (init.rs + fork_first.rs) Also fixes pre-existing incorrect test assertion in test_pty_config_claude_with_teams which claimed workers should NOT have --session-id (they always do). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
FactoryCore::new was calling set_worker_cli and set_worker_effort but missing set_worker_model, leaving worker model config silently ignored when using the dynamic-spawn path. Detected and applied by cas-code-review autofix loop (round 1). Refs: cas-63d9 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
pippenz
added a commit
that referenced
this pull request
May 12, 2026
P2 #4 — show_help was missing from alt_screen_scroll_input guard. When the help overlay is open and the focused pane is in alt-screen, wheel events were incorrectly forwarded to the inner PTY. Fix: add `|| self.show_help` to the suppression guard and update the doc comment to enumerate all overlay conditions. P2 #5 — PgUp (ESC [ 5 ~) and PgDn (ESC [ 6 ~) were not forwarded in alt-screen mode; the generic ESC handler only consumed the first byte (bare 0x1b) and left the remaining [5~ as stray ASCII. Fix: add a dedicated 4-byte sequence check in client_input.rs before the arrow-key check. In alt-screen mode, the full native sequence is forwarded via mux.send_input; outside alt-screen, handle_scroll_up/down is called (same behavior as mouse wheel). Satisfies AC #1 ("same on PgUp"). Added FactoryApp::for_test() — #[cfg(test)] minimal constructor that builds a FactoryApp without spawning PTYs or opening files, enabling unit tests for guard logic at the FactoryApp layer. Tests added: - sidecar_and_selection::tests (11 new unit tests): - Baseline: returns Some when guard clear + pane in alt-screen - show_help guard blocks forwarding (P2 #4) - MC + mc_focus==None guard blocks forwarding (P1 #1 regression) - MC + mc_focus==Workers guard blocks forwarding - PgUp/PgDn dispatch fires when alt-screen active (P2 #5) - PgUp/PgDn fall through to handle_scroll when NOT in alt-screen - Wheel scroll no-regress when not in alt-screen (AC #3) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
pippenz
added a commit
that referenced
this pull request
May 13, 2026
The `FactoryApp::for_test()` constructor itself was already shipped in commit 53a7bf4 (cas-d5fa P2 fix) on 2026-05-07 — the same day cas-11b0 was opened. The structural AC bullets (cfg(test)-gated, Mux::new vs Mux::factory, in-memory stubs, DirectorStores=None, all dialogs=false, sidecar_focus=None, factory_view_mode default) are all satisfied; 8 tests in `sidecar_and_selection` already exercise the constructor and pass on main. The one remaining AC bullet — "document which fields need special handling (Notifier, PaneGrid, DirectorEventDetector, etc.)" — was the only meaningful unfinished work. Expand the docstring from 3 lines to a structured field-handling note covering the ~10 fields with non-obvious construction reasoning: `mux` (new vs factory), the DirectorEventDetector `.initialize` sequence, `director_stores=None` contract, the empty DirectorData literal, theme/notifier inert defaults, the cas_dir/project_dir placeholder warning, and the "keep terminal_cols/rows in sync with the Mux dimensions" pitfall. Also adds the canary clause: "If you add a field to FactoryApp, you must also add it here — the test constructor will fail to compile otherwise, which is the intended canary." This makes the rolling cost of `FactoryApp::for_test` legible to anyone adding new fields. No code changes; docs-only. All 25 ui::factory::app tests still pass. This is commit #4 on factory/daring-swan-93 (dc95458 cas-f645 + 16f5ba8 cas-ec8f main + b48706c cas-ec8f amendment + this), per supervisor instruction not to rebase. Cherry-pick split at merge time still works.
pippenz
added a commit
that referenced
this pull request
May 13, 2026
…screen cas-72c3 was the deferred test rider for cas-d5fa, blocked on cas-11b0 (FactoryApp::for_test constructor). Per cas-11b0's close note, the for_test() constructor + 8 unit tests at the FactoryApp layer already shipped in 53a7bf4 (same day cas-d5fa landed), covering: - AC #1 (MC + mc_focus==None + alt-screen → no forward): scroll_blocked_by_mc_focus_none - AC #2 (show_help + alt-screen → no forward): scroll_blocked_by_show_help - AC #4 (non-alt-screen no-regress): wheel_scroll_no_regress_when_not_in_alt_screen The genuinely uncovered slice was AC #3 — the daemon-dispatch path in `client_input.rs` lines 157-187 that maps `MouseScrollUp` to either `diff_scroll_up()`, `mux.send_input(SCROLL_UP_ARROWS)`, or a no-op, depending on `show_changes_dialog` and `handle_scroll_up()`'s return. That branch lives inside a deep `tokio::select!` and can't be exercised directly from a unit test without spinning up the full client loop, so this commit pins the three contracts the daemon relies on instead: 1. scroll_arrow_consts_have_exact_byte_shape_cas_72c3 Asserts SCROLL_UP_ARROWS == `ESC[A` × SCROLL_LINES and SCROLL_DOWN_ARROWS == `ESC[B` × SCROLL_LINES. Pre-cas-72c3 the only structural assertion (line ~37) checked length, not bytes; a typo could silently break wheel forwarding without firing any guard. 2. scroll_changes_dialog_blocks_alt_screen_forwarding_cas_72c3 With both show_changes_dialog AND alt-screen active, `handle_scroll_up` must return `Done` (not `AltScreen`). That's the post-condition the daemon's outer `if self.app.show_changes_dialog` branch relies on to early-return without forwarding arrows to the PTY. Pins both up and down directions. 3. daemon_dispatch_table_for_mouse_scroll_up_cas_72c3 Table-driven mirror of the daemon's three-way decision (`diff`, `alt`, `noop`), asserted against five FactoryApp state shapes. Any future drift in either the daemon dispatch or `handle_scroll_up`'s return contract fails the test row that no longer maps correctly. All 11 sidecar_and_selection tests (8 pre-existing + 3 new) pass. Test-only diff; no production code touched. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
Summary
reasoning_effortconfig per-role (supervisor/worker) through the full factory spawn pipeline, mirroring the existingmodelpatternMux::factory()(static init) andFactoryCore::spawn_supervisor/spawn_worker()(dynamic)xhighfor supervisor,highfor worker when no config setFactoryCore::newnow also callsmux.set_worker_model()for symmetry withset_worker_effort()af5358a(v2.11.0); all new test call sites from origin/main updated for new 9-argPtyConfig::claude/codexsignaturesFiles changed
crates/cas-pty/src/pty.rs— addedeffort: Option<&str>param;unwrap_ordefaults toxhigh/high; 10 new test call sites updatedcrates/cas-mux/src/pane/mod.rs— forwarded effort throughPane::supervisor()andPane::worker()crates/cas-mux/src/mux.rs— addedsupervisor_effort/worker_efforttoMuxConfig; storedworker_effortinMuxfor dynamic spawncrates/cas-factory/src/config.rs— addedsupervisor_effort/worker_efforttoFactoryConfigcrates/cas-factory/src/core.rs— wiredworker_effortandworker_modelthroughFactoryCore::new()cas-cli/src/cli/factory/mod.rs,daemon.rs— reads fromLlmConfig::reasoning_effort_for_role()cas-cli/src/ui/factory/app/init.rs,daemon/fork_first.rs— forwarded throughMuxConfigTest plan
pty.rs: custom effort, supervisor default (xhigh), worker default (high)cas-ptytests passcas-muxandcas-factorypackages compile and test cleancascrate compiles (warnings only, pre-existing)Closes: cas-63d9
🤖 Generated with Claude Code