feat(harness): thinking default-collapsed preference toggle via Leader+t (plan #742) - #754
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Summary (re-implementation)This is plan #742's fresh PR against Gates (all green): No TS changed → vitest/typecheck/DI-cost N/A. No cap added/changed; no Production mutate. Plan #742 marked IMPLEMENTED. Next: adversarial review on this head |
btipling
left a comment
There was a problem hiding this comment.
Adversarial review — PR #754
Verdict: PASS WITH NOTES
Repo: btipling/invincible
Scope: main ← plan/thinking-default-collapsed · 9 files · Leader+t thinking default-collapsed (#742)
Lenses run: L1, L3, L5, L6, L8, L9 (skip: L2 no secrets/API; L4 no workflow; L7 no clone bind)
AGENTS.md read: yes · docs/feature-divide.md read (preference is Wasm in-memory; no meta / protocol / DOM)
The shape is right: shouldRenderFull = operator_open OR (NOT default_collapsed AND isActiveTurnFull) is the single policy, preference default ON, Busy pin gated in paint (pinned_active = is_active and !default_collapsed) so mid-Busy expand still writes thinking_open_l1. Leader+t is a short-circuit arm in match (same class as ?) — the KEY_TABLE row would never fire without it. fromDvui already has .t (Ctrl/Cmd+T stay reserved → .browser, disarm, no toggle). Dispatch flips then disarmLeader(). Reset on New/Clear/hydrate returns ON. Caps unchanged (KEYMAP_MAX 64, table still well under). Overlay exhaustive switch compiles. Docs qualify the old “Busy stays fully expanded” claim.
Tests lock both preference values + committed rows + leader+t / plain t / Ctrl+T reserved. No dual DOM cheatsheet.
Findings
| Sev | Lens | Finding | Break scenario | Refutation attempt | Confidence |
|---|---|---|---|---|---|
| Nit | L8 | thinking_collapse.zig file header still locks “Busy → every current-turn thinking row FULL” (#424). Product default is now the opposite. |
Next collapse agent “restores” the Busy pin from the module comment and fights #742. | shouldRenderFull body + tests are the real API; header is stale prose. |
high |
Residual risk
- Test 5b names “disarms / never inserts
t” but only assertsmatchoutcome; disarm + handled-mark live inkeymap_dispatch(not intest-rich). - Overlay greys the new row unless
leader_pending(same as leader+?) — technically correct, easy to misread as “unavailable”. - Zig
test-rich/ wasm32build-harnesswere not re-run in this review environment.
Merge guidance
- PASS WITH NOTES: safe to merge from this attack; nits optional
- Do not persist the preference in session
meta/ a newinv_*in this PR - Do not add a DOM
windowkeydown for Leader+t - Do not silently change
KEYMAP_MAX/LEADER_WINDOW_MS
What was not attacked
Host zig build test-rich / wasm32 build-harness, live canvas (Leader+t mid-Busy expand/collapse, New session reset), Production harness artifact deploy.
|
Addressing adversarial-review Nit (L8) from the PASS WITH NOTES review on Change
The New head: CI ( Per merge skill §0a, the head moved ( |
btipling
left a comment
There was a problem hiding this comment.
Adversarial review — PR #754
Verdict: PASS WITH NOTES
Repo: btipling/invincible
Scope: main ← plan/thinking-default-collapsed · 9 files · Leader+t thinking default-collapsed (#742)
HEAD: ec5f709 (header-only follow-up after 2c215e5)
Lenses run: L1, L3, L5, L6, L8, L9 (skip: L2 no secrets/API; L4 no workflow; L7 no clone bind)
AGENTS.md read: yes · docs/feature-divide.md read (preference is Wasm in-memory; no meta / protocol / DOM)
Re-review. Prior Nit (L8: thinking_collapse.zig header still locked “Busy → FULL”) is fixed. Header now matches shouldRenderFull = operator_open OR (NOT default_collapsed AND isActiveTurnFull) and the product default ON.
Logic is unchanged from the previous attack and still holds: leader+t is a match short-circuit (same class as ?); Ctrl/Cmd+T stay reserved; dispatch flips then disarmLeader(); paint pinned_active is preference-gated so mid-Busy expand writes thinking_open_l1; reset on New/Clear/hydrate returns ON; overlay exhaustive switch compiles; caps unchanged; tests lock both preference values + leader+t / plain t / Ctrl+T reserved. No dual DOM cheatsheet.
Findings
None on ec5f709.
Residual risk
- Test 5b names “disarms / never inserts
t” but only assertsmatchoutcome; disarm + handled-mark live inkeymap_dispatch(not intest-rich). - Overlay greys the new row unless
leader_pending(same as leader+?). - Zig
test-rich/ wasm32build-harnesswere not re-run here (comment-only delta).
Merge guidance
- PASS WITH NOTES: safe to merge from this attack
- Do not persist the preference in session
meta/ a newinv_*in this PR - Do not add a DOM
windowkeydown for Leader+t - Do not silently change
KEYMAP_MAX/LEADER_WINDOW_MS
What was not attacked
Host zig build test-rich / wasm32 build-harness, live canvas (Leader+t mid-Busy expand/collapse, New session reset), Production harness artifact deploy.
plan #742 — thinking default-collapsed toggle via Leader+T
Re-implementation of plan #742 against current
main. The original implementation (#751) was stacked onplan/harness-keymap(parent plan #741) and was auto-closed without merging when #746's merge deleted that base branch. Those keymap seams are now onmain, so this PR re-implements the plan directly ontomain(Zig + docs only).Fixes #742
Refs #741 (parent keymap)
Change (harness + docs only)
thinking_collapse.zig(pure, host-testable) —shouldRenderFullnow threadsdefault_collapsedas an explicit boolean parameter:shouldRenderFull = operator_open OR (NOT default_collapsed AND isActiveTurnFull).Default ON relaxes the Busy pin; OFF restores today's behavior.
ui/state.zig—thinking_default_collapsed = true, reset to ON inresetTranscriptScroll()(same lifetime/reset site ashelp_overlay_open/leader_armed). In-memory only; no sessionmeta/ protocol carrier.ui/thinking.zig— threads the live preference; theis_active"expanded = true" pin is gated (pinned_active = is_active and !default_collapsed), so when ON an active row's click routes through the normal set-toggle intothinking_open_l1(expander works mid-Busy). No secondif (is_active)pin left that ignores the bool.keymap.zig(Action.thinking_default_toggle+KEY_TABLErowthinking_default_toggle+ a leader-pendingtarm inmatch, mirroring the?arm: handled, disarms, never insertst),ui/keymap_dispatch.zig(Handlers.toggleThinkingDefaultseam + arunActionarm that flips + disarms the leader),ui/help_overlay.zigrowChordcase (exhaustive switch — compile-forced,Thinking default collapsed/Leader Space, then t),ui.zigseam + dispatch wiring.docs/harness-limits.md(keyboard row: Leader thent; Thinking collapse row: Busy pin now preference-gated),docs/agent-stream.md(qualified the absolute "active Busy turn keeps thinking fully expanded").Verification (Zig gates, all green)
zig fmt --checkzig build test-richzig build test-rich-invariantszig build harness -Doptimize=Debugbuild-harnessCINo TS files touched → vitest / typecheck / DI-cost gates N/A (per the layer rule). No cap added/changed. No Production mutate;
build-harnessis the existing release gate.Per the plan's testing lock (#1–6, 5b, 8; #7 compile-forced): tests 1–3 (both preference values + committed rows), 5/5b (leader+t → toggle, disarm, no prompt insert), table-within-cap.