feat(pi): keep failed branch-outcome reads visible under Calm - #3106
Open
zachlandes wants to merge 9 commits into
Open
zachlandes wants to merge 9 commits into
zachlandes wants to merge 9 commits into
Conversation
* Calm hid every other tool row while `fm_branch_outcomes` dumped the outcome store's raw JSONL into the transcript, reproduced against real Pi 0.84.3: with Calm on, a built-in bash row vanished in the same turn where the outcome read printed every record verbatim. * Taught Calm this tool instead of adding a second hiding path. The row now reaches Calm's existing visibility owner the same way the watcher tool does, over Calm's published presentation event, so Calm stays the one place that decides what the transcript shows. * Kept failure visible. An outcome the branch already handled collapses with the row, while a failed read, a captain-verdict outcome, and any output Calm does not recognize as the store's records each survive as one dim line under the branch's own glyph, so a fleet failure is never hidden. The store's own verdict field is the actionable marker; no keyword guessing. * Preserved stock and export rendering. Collapsing needs self shell rendering, so the slots reproduce Pi's own fallback, including the unexpanded clip and expand hint; a Calm-off transcript came back byte-identical to the pre-change one at 3 and at 18 records, and the HTML export carries every record. * Reviewed the other Calm surfaces (assistant layout, operational-user layout, working ship, visibility policy): none are touched, and their suite still passes.
Confidence Score: 5/5The PR appears safe to merge under Kun’s merge authority, with no actionable defects identified. The changed renderer preserves stock and export paths, collapses only recognized routine outcomes, and retains failed, captain-relevant, and unrecognized output. Reviews (1): Last reviewed commit: "no-mistakes(document): Refresh Calm bran..." | Re-trigger Greptile |
* Kept main's stock preview and expansion checks in the outcomes renderer test and moved the Calm-on collapse check onto a routine store record, since unrecognized lines now stay visible by design. * Taught the Calm recognizer the status-provenance record shape that bin/fm-branch-outcome.sh gained in kunchenguid#3495; without it every routine outcome read as unrecognized and stayed on screen under Calm. * Folded the branch-outcome exception into main's restructured Calm, supervision-branch, and verification docs.
zachlandes
changed the title
…used two of them, and both are now fixed. 1. **fm-live-gate.test.sh (shard 7), caused by this PR.** Rule it broke: every guard in the live-harness family has to start with the shared `fm_live_gate`, so that `FM_LIVE=0` switches them all off together. The new `tests/fm-calm-branch-outcomes-live-e2e.test.sh` used its own on/off check and its own pi/tmux checks instead. I replaced those with `fm_live_gate opt-in FM_CALM_BRANCH_OUTCOMES_LIVE_E2E pi tmux`, which keeps the "refuse to pass having checked nothing" behaviour when the guard is asked to run. It was the only file in this diff with its own gate. `tests/fm-live-gate.test.sh` now passes: all 40 live guards skip together under `FM_LIVE=0`. 2. **fm-pi-branch-responsiveness-live-e2e.test.sh (shard 6), caused by this PR.** Rule it broke: any test that copies `fm-branch-supervision.ts` into a fixture must also copy every `./lib` file it imports. This PR added an import of `./lib/fm-calm-branch-outcomes.ts`, but the fixture didn't copy that file, so Pi couldn't load the extension and its TUI never drew. I checked every test that copies the extension. The same gap was in `tests/fm-pi-branch-live-e2e.test.sh`, which CI didn't catch because it only runs when asked, so I fixed both. `fm-pi-codex-native` loads the extension straight from the repo, so it needed no change. The responsiveness guard now passes locally against the real Pi 0.87.1: "ok - supervision outcome delivery keeps the real Pi 0.87.1 TUI echoing keystrokes at its unloaded floor". 3. **fm-remote-secondmate-relaunch.test.sh (shard 6), not caused by this PR.** It fails with "could not arm the PR poll fixture for the relaunch-ordering test". That part of the test was added upstream in e789e52 (kunchenguid#5583). It fails the same way when run on a clean export of the base commit ea7c7f7, with none of this branch's changes, and this PR touches nothing it uses. I made no change for it. While debugging number 3, a temporary `sed` edit I made emptied `bin/fm-pr-check.sh` in the worktree. I restored it with `git checkout`, and it is not part of the changes. Also checked: `shellcheck -x` is clean on the three changed test files, and `tests/fm-calm-branch-outcomes.test.sh` still passes
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.
Intent
#3024 made Calm collapse the
fm_branch_outcomestool row entirely, so the raw outcome-store JSONL no longer floods the transcript.The collapse is unconditional, which means two things that need a human also disappear under Calm: a read of the outcome store that failed, and output that is not a store record at all.
The worst case is someone asking what happened in the fleet, the store read failing, and Calm rendering that failure as an empty transcript.
This keeps exactly those two cases on screen, each as one dim line carrying the supervision branch's sailboat glyph, while a read made only of well-formed store records still collapses to nothing.
Captain-verdict outcomes are deliberately not part of this exception.
Since #3312 the supervision branch delivers every captain outcome as its own exact-once visible transcript entry, so repeating it on each Calm read of the store would duplicate what the captain already sees.
The recognizer is strict on purpose, because the safe direction is to show too much rather than too little.
A line is hidden only when it matches one of the exact record shapes
bin/fm-branch-outcome.shvalidates: the legacy six-key row, the row with a booleansilent, or the row that also carries thestatusEndpoint/statusIdentprovenance added in #3495, each with integralseqandepochand aroutineorcaptainverdict.Anything else is shown byte-for-byte, so a future change to the store format degrades into visible output instead of silent suppression; if the store grows another field, extend
OUTCOME_KEY_SETSin.pi/extensions/lib/fm-calm-branch-outcomes.tsalongside it.The collapse decision still runs through Calm's existing visibility owner, Calm-off rendering keeps #3261's stock preview and expansion behaviour, and HTML export is untouched.
Verification:
tests/fm-calm-branch-outcomes.test.shpins the decision against records written by the real store script, with no harness.The opt-in guard
FM_CALM_BRANCH_OUTCOMES_LIVE_E2E=1 tests/fm-calm-branch-outcomes-live-e2e.test.shdrives the realpi0.87.1 binary in tmux and asserts what it paints: handled records collapse, a failed read stays visible, Calm-off keeps the stock row, and/exportstill contains every record.The Pi extension suites and the strict typecheck against Pi 0.87.1 also pass.
The one failing CI job (Behavior portable serial 6,
tests/fm-remote-secondmate-relaunch.test.sh) is a pre-existing failure on main since #5583 added that test: https://github.com/kunchenguid/firstmate/actions/runs/36212602588/job/108322127185Written with AI assistance.
What Changed
fm_branch_outcomestool result, it now checks what that result contains, using the new pure module.pi/extensions/lib/fm-calm-branch-outcomes.ts. A read made up only of valid store records, or the empty-store text, still disappears with the row. A failed read, and any line that doesn't pass the store's record key-set and type checks, now stays on screen as a dim line with the branch's sailboat glyph. Before this, both were silently hidden.tests/fm-calm-branch-outcomes.test.sh, which tests that decision against real store records without a harness. Addedtests/fm-calm-branch-outcomes-live-e2e.test.sh, a live check gated onFM_CALM_BRANCH_OUTCOMES_LIVE_E2E, which runs the realpibinary in tmux. It checks four things: the row with Calm off, the collapse with Calm on, the export, and a failed read. Both tests are registered inbin/fm-test-run.sh. The existing branch-extension and type-check fixtures now copy the new module and use valid routine records.docs/calm.md,docs/calm-mode-feasibility.mdanddocs/pi-supervision-branch.mdto describe this exception and name the module that owns it. Recorded the dated live-run output against Pi 0.87.1 indocs/verification/runtime-backends.md.Risk Assessment
Testing
Ran the portable regression and the Pi branch-extension suite (both green). Ran the shipped opt-in live guard against real Pi 0.87.1 (4/4 ok). Then ran an extended live copy with two extra adversarial cases, unrecognized output and a corrupt store (6/6 ok). Pane snapshots and the HTML export are saved as evidence. Every live scenario passed. The module-level single-line rendering check is recorded as untested live, because it was only a pure-function check.
Evidence: Live guard as shipped (real Pi 0.87.1)
Source: Live guard as shipped (real Pi 0.87.1)
Evidence: Extended live guard output
Source: Extended live guard output
Evidence: Calm on: complete store read collapses to nothing (pane)
Source: Calm on: complete store read collapses to nothing (pane)
Evidence: Calm on: failed read stays visible with ⛵ (pane)
Source: Calm on: failed read stays visible with ⛵ (pane)
⛵ could not read the outcome store: fm-branch-outcome.sh exited 127: ... No such file or directoryEvidence: Calm on: corrupt store failure stays visible (pane)
Source: Calm on: corrupt store failure stays visible (pane)
⛵ could not read the outcome store: fm-branch-outcome.sh exited 1: error: refusing read because the outcome store is malformed or non-sequentialEvidence: Calm on: unrecognized line kept byte-for-byte (pane)
Source: Calm on: unrecognized line kept byte-for-byte (pane)
⛵ OUTCOME_V2|task-77|verdict=escalate|CALM_OUTCOMES_FUTURE_FORMAT keeps spacingEvidence: Calm off: stock row with preview clip (pane)
Source: Calm off: stock row with preview clip (pane)
Evidence: Calm-on session HTML export
Source: Calm-on session HTML export
Evidence: Portable regression output
Source: Portable regression output
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
tests/fm-calm-branch-outcomes-live-e2e.test.sh:231- The Calm-off live case assertsCALM_OUTCOMES_ROUTINE_14(line 15 of the 16-record listing) is on screen, and the comment at lines 167-168 says the upstream renderer "shows the complete sanitized store listing". This merge brings in upstream f66be0f (fix(pi): restore Pi 0.84.4 renderer compatibility #3261, 2026-08-28), which addedgetStockOutcomesPreviewLines. That function clips the Calm-offfm_branch_outcomesresult to Pi's stock collapsed preview on Pi >= 0.84.4. The upstream regression intests/fm-pi-branch-extension.test.sh(theOUTCOME_TWELVEcollapsed-preview check, PI_STOCK_RENDER_FLOOR=0.84.4) already asserts that a 12-line result is clipped with a "more lines ... to expand" hint. On the installed Pi 0.87.1, runningFM_CALM_BRANCH_OUTCOMES_LIVE_E2E=1 tests/fm-calm-branch-outcomes-live-e2e.test.shas the docs instruct would failtest_calm_off_keeps_the_stock_rowbecause line 15 is outside the preview. Fix: assert a record inside the preview window, such as the first captain rowtask-12raw JSON and"verdict":"routine"(already asserted), optionally plus the expansion hint. Correct the comment. The dated record indocs/verification/runtime-backends.md:2232-2244(Pi 0.84.3, 2026-08-24) predates this merge and fix(pi): restore Pi 0.84.4 renderer compatibility #3261, so re-run the guard and refresh it on the current Pi rather than keep that output..pi/extensions/lib/fm-calm-branch-outcomes.ts:99- Intent: "making sure we arent repeating someone else's valid fix". This merge brings in upstream 5466394 (fix(pi): deliver captain outcomes as deterministic transcript entries #3312, 2026-09-01), which now delivers every captain-verdict outcome as a durable, exact-once visible transcript entry (VISIBLE_OUTCOME_ENTRY_TYPE, fm-branch-supervision.ts ~975). Routine notes already render as⛵ task: summary(deliverRoutineOutcome, ~988). The recorded rebase decision justified keeping captain-verdict rows because upstream hid them "entirely". That rationale was written on 2026-08-26, before fix(pi): deliver captain outcomes as deterministic transcript entries #3312 existed, and no longer holds: the captain already sees each captain outcome in the transcript. With this branch, every Calm read of the store (list --recent 20) repeats each captain row in the window as a second⛵ task: summaryline, including outcomes processed long ago. The failed-read and unrecognized-output parts are still not covered upstream. Decide whether to narrow the Calm exception to failed reads and unrecognized output (drop the captain-verdict branch at lines 99-103 plus its tests and doc wording in docs/calm.md:23 and docs/calm-mode-feasibility.md:230), or to keep captain rows deliberately despite fix(pi): deliver captain outcomes as deterministic transcript entries #3312..pi/extensions/lib/fm-calm-branch-outcomes.ts:21-calmBranchOutcomeAttentionnow returnsglyph: trueon every path (lines 86, 96, 101). So theglyphfield onCalmBranchOutcomeLineand the renderer'sglyph === falsebranch (.pi/extensions/fm-branch-supervision.ts:2181) are dead code. The recorded rebase decision prescribed that branch, so this is noted only; it could be dropped if that decision is revisited.🔧 Fix applied.
2 infos still open:
.pi/extensions/lib/fm-calm-branch-outcomes.ts:21-calmBranchOutcomeAttentionnow returnsglyph: trueon every path (lines 86, 96, 101). So theglyphfield onCalmBranchOutcomeLineand the renderer'sglyph === falsebranch (.pi/extensions/fm-branch-supervision.ts:2181) are dead code. The recorded rebase decision prescribed that branch, so this is noted only; it could be dropped if that decision is revisited.docs/calm.md:23- The newfm_branch_outcomessentence was inserted between the sentence that introduces "the shared preservation rule above" (line 22) and "Pi applies that rule independently to each text block…" (line 24). As a result, "that rule" now reads as if it refers to the branch-outcome collapse instead of the working-note preservation rule. The fix is to move the new sentence below the working-note run, for example after the "…and/exportartifacts." sentence at line 26, so the preservation-rule paragraph stays contiguous.✅ **Test** - passed
✅ No issues found.
bash tests/fm-calm-branch-outcomes.test.sh(portable regression, real store records)bash tests/fm-pi-branch-extension.test.sh(46 ok, including the fm_branch_outcomes Calm hide, Calm-off and export case)FM_CALM_BRANCH_OUTCOMES_LIVE_E2E=1 bash tests/fm-calm-branch-outcomes-live-e2e.test.shas shipped, against real pi 0.87.1Extended copy of the live guard (temp dir, since removed) that saved pane snapshots and added two adversarial real-Pi cases: a future-format unrecognized line from a fixture outcome script, and a corrupt real store file✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.