Skip to content

fix(chat): move forward to unanswered questions on pi-ask and omp question cards - #2508

Merged
devswha merged 10 commits into
mainfrom
fix/question-card-forward-steps
Nov 1, 2026
Merged

devswha merged 10 commits into
mainfrom
fix/question-card-forward-steps

Conversation

@devswha

@devswha devswha commented Oct 31, 2026 •

Copy link
Copy Markdown
Owner

What was wrong

Follow-up to #2506. The Chat question card could not move forward to an unanswered question, or between questions at all, where the agent's own terminal UI can:

Harness The agent itself (source) Card before Card after
pi-ask tabs (rpiv-ask-user-question 2.13.9) Right moves on without answering (state/key-router.ts:130-145, state/state-reducer.ts:127-146) Back and earlier tabs only Next too (→, each move seen)
pi-ask ask_user (@eko24ive/pi-ask 1.2.0) moveTab cycles every question (src/state/navigation.ts:30-42, src/ui/controller.ts:281-285) earlier or answered steps only any question, and Next; drafts kept, no keys sent
omp Ask (pi-tui 18.13.2) Tab/Shift+Tab between questions (chrome/selector-helpers.ts:75-90, overlays/ask-dialog.ts:739-742,1483-1486) one question, no steps steps, Back and Next between questions
OpenCode native form not established from the installed build Back only unchanged

What changes

  • pi-ask tabs: Next where the next tab is itself navigable.
  • pi-ask ask_user: every question is reachable, as pi-ask's own moveTab allows. The bridge saves the card's draft on a move and sends no terminal keys.
  • omp: steps come from its styled tab strip, and only from a complete, unambiguous one. Moves use Tab and Shift+Tab through the shared guarded, deadline-bound move keys from fix(chat): move between a form's questions from the card without answering #2506, and only on a question whose drawn text and options are the pane's single open ask call. The identity is session + call id + full arguments.
    • The review draws only headers and answers, so it offers no move, and no step goes onto it.
    • The card's id does not depend on whether the colours or the call could be proven on that read.
    • Without colour proof or a matching call, the card offers no move, and ordinary answers keep working.
  • OpenCode: its rule for leaving a required field empty could not be established from the installed binary, so forward moves stay off there.

Validation

  • Failing-first tests for each harness, and mutation checks (22 cases) where a fix could be removed.
  • 24 rendered native omp states checked against its own handlers (dark and light, 120 and 60 columns).
  • bun run check fast green on the head rebased onto main (10129 pass, 0 fail).
  • Reviews: Codex gpt-6.1-terra (fast, xhigh), three passes, every finding fixed with a failing-first test. The lead's own review of the whole diff found nothing blocking.
  • Not run: check full, Windows VM (no Windows-specific path).

devswha and others added 3 commits November 1, 2026 06:47
Co-authored-by: sisyphus-dev-ai <sisyphus-dev-ai@users.noreply.github.com>
Co-authored-by: sisyphus-dev-ai <sisyphus-dev-ai@users.noreply.github.com>
… keep its id

Terra's review of the follow-up:
- omp's review draws headers and answers alone, which another call with the same headers draws
  as well: it offers no move, and no step goes onto it. Moves stay between questions whose drawn
  text and options are the pane's one open call.
- The card's id is read without its steps: colours or a call found on one read and not the next
  leave the same card, so an answer from it is not refused as stale.
@coderabbitai

coderabbitai Bot commented Oct 31, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • Trigger review
Summary

Summary by CodeRabbit

  • New Features
    • Navigate forward to unanswered questions in Pi ask flows, including after returning to an earlier question.
    • Move between supported OMP Ask questions and review screens using question-card navigation.
  • Bug Fixes
    • Unsent edits remain visible while navigating and are cleared or sent before navigation proceeds.
    • Navigation is unavailable when the question tabs or matching open call cannot be verified, or when the current screen does not support the requested step.

The change adds forward navigation to unanswered Pi Ask questions and adds guarded tab navigation for OMP question and review cards. OMP navigation depends on styled tab evidence and a matching open ask call. Unsent edits continue to block step navigation.

Changes: Question-card navigation

Layer / File(s) Summary
Pi Ask forward navigation
server/pi-ask-bridge.ts, server/dialogs/pi-ask.ts, server/dialogs/pi-ask-route.test.ts, server/dialogs/pi-ask-user-route.test.ts
Pi Ask cards mark unanswered questions as navigable and expose Next when another question can be reached. Navigation accepts one action without answer fields, and tests cover unanswered questions, validation, drafts, and cancellation.
OMP tab detection and call matching
server/dialogs/omp.ts, server/omp-ask.ts, server/question-steps.test.ts
OMP cards expose tab steps only when the styled strip is complete and unambiguous. Navigation also requires one matching open ask call and a matching displayed question. Tests cover review states, invalid evidence, key movement, and changed or stale prompts.
Prompt navigation integration and edit handling
server/prompt.ts, server/prompt.test.ts, src/lib/promptAnswer.ts, src/lib/promptAnswer.test.ts, changelog.d/question-card-forward-steps.md
Prompt handling accepts Next for supported navigation metadata and binds OMP prompts to open asks. OMP joins the existing unsent-edit navigation check. The changelog records these behaviors.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Possibly related issues

  • devswha/ovis#2465 — This change extends the shared question-card navigation contract, but its new tests cover Pi Ask and OMP, not completion of the Claude Code navigation requirement.

Possibly related PRs

  • devswha/ovis#1714 — Adds the Pi question-card tabs and native navigation that this change extends to unanswered questions.
  • devswha/ovis#2506 — Adds the shared Next contract and guarded navigation that this change applies to Pi Ask and OMP Ask.
  • devswha/ovis#2220 — Matches OMP Ask cards to an open ask call and displayed question, which this change uses when enabling tab navigation.

Suggested reviewers: 767251930

Merge Risk: Minimal · previous review 53654

No demonstrated behavior change blocks merging. The redundant test-fixture branch can be cleaned up separately.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 53.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly identifies the primary change: enabling forward navigation to unanswered questions for pi-ask and OMP question cards.
Description check Passed The description explains the problem, resulting behavior, implementation scope, validation results, contract-related coverage, and skipped checks. It uses “What was wrong” and “What changes” instead o…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description explains the problem, resulting behavior, implementation scope, validation results, contract-related coverage, and skipped checks. It uses “What was wrong” and “What changes” instead of the template’s “Change” heading, but the required information is present.

Full details: Docstring Coverage

Explanation

Docstring coverage is 53.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 15 functions across 11 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches
Generate docstrings
  • Commit to this branch
  • Create a new PR
Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

@devswha

devswha commented Oct 31, 2026

Copy link
Copy Markdown
Owner Author

Review by the merge pass at 53654d7 (Codex Terra; the Claude review is still running and will be added). Not clean, so it moves to 0.4.23 per the owner's rule:

  1. P1, reproduced: server/omp-ask.ts:26 leaves the call identity out of the card id, so two distinct omp calls drawing the same question get the same id and both authorize navigation; a stale card can send Tab or Enter to a different call. Include the call id in the card id and refuse a move when it changed.
  2. P1, not reproduced end to end: server/prompt.ts:720 checks only foreground ownership for omp before the key; a replacement form opened between the last read and the send could take it. Re-check the call identity in the read right before each key, as fix(chat): move between a form's questions from the card without answering #2506 does.
  3. P2, reproduced by the parser tests: omp review navigation is unavailable (server/omp-ask.ts:25 disables moves from review and server/dialogs/omp.ts:144 excludes moving to it), which the changelog claims.

Terra's parser and frontend tests passed (79); the route tests could not run in its sandbox.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additional comments: 1
server/prompt.test.ts: 1

4197-4197: Trivial · Maintainability & Code Quality · Quick win

Unverified finding
Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.

Remove the unreachable pane.get handler.

Line 4194 already handles pane.get and returns first, with no scroll field. The new handler on Line 4197 never runs. The fixture still reports no scroll, so the change has no effect.

If a test needs scroll.offset_from_bottom, edit Line 4194. Otherwise, remove Line 4197. Line 4193 documents the missing scroll as intentional.

Proposed fix
-        if (request.method === "pane.get") return answer({ pane: { pane_id: "p_1", scroll: { offset_from_bottom: 0 } } });

Review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between f5370d5 and 53654d7.

Files selected for processing: 12
  • changelog.d/question-card-forward-steps.md
  • server/dialogs/omp.ts
  • server/dialogs/pi-ask-route.test.ts
  • server/dialogs/pi-ask-user-route.test.ts
  • server/dialogs/pi-ask.ts
  • server/omp-ask.ts
  • server/pi-ask-bridge.ts
  • server/prompt.test.ts
  • server/prompt.ts
  • server/question-steps.test.ts
  • src/lib/promptAnswer.test.ts
  • src/lib/promptAnswer.ts

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.

…ch key

The merge pass's review of #2508 found three things:
- An omp card's id left out its call, so a card read under one ask call could send Tab or Enter to the
  next call omp drew with the same questions. A card read under the pane's one open ask call now
  names that call in its id.
- Between the last read and a key, nothing checked the call again, and the answer path's look before
  a key had no bound. Before each key the call must still be the pane's one open ask call, and the
  look (omp's foreground check included) is held to the window of the read it follows.
- The changelog said omp moves reach the review, which the code never offers. The changelog now says so.
@devswha

devswha commented Oct 31, 2026

Copy link
Copy Markdown
Owner Author

Claude review by the merge pass at 53654d7 (for the 0.4.23 round):

  • HIGH, reproduced (confirms Terra's P1): a stale omp card can move a different call. ompStepPrompt (server/omp-ask.ts:26-32) binds moves to the call but the card id stays screen-only (server/dialogs/omp.ts:297), and the POST compares only that id (server/prompt.ts:1222). Repro: a card read under call A, call B opens with the same questions, the ids are equal, and the stale step_next returned 200 and sent tab.
  • MEDIUM, reproduced: when the call match drops mid-move, server/dialogs/step-navigate.ts:79-81 sends nothing more but keeps reading until the 4 s deadline; the test "omp: a different strip after the first key stops the move" passes no short deadline and runs 4008 ms.
  • MEDIUM: server/question-steps.test.ts:421 (OmO Next got 400) failed once in three runs of the touched files together (it passes alone); make it independent of shared state.
  • MEDIUM: the changelog says the card moves between questions and their review, but no move onto or off omp's review is allowed.
  • LOW: the added pane.get line at server/prompt.test.ts:4197 is unreachable.

Holds: a move is its own asking, the 4 s deadline covers every read and key, Next sends only Tab or → and never Enter, the shell and other-agent checks work, pi-ask ask_user only edits the server's draft (no keys, no new input path), OpenCode is untouched. The card visibly changes (omp step strip with Back/Next; Next on pi-ask tab and ask_user cards), so it needs screenshots for the owner.

… lookup, and stop an unbound move at once

Merge-pass and review findings on #2508:
- A move whose next screen shows omp's strip with no call binding it (omp asked again, or another
  strip) now stops at once instead of reading until the 4 s deadline.
- A card binds the pane's open ask call only when the screen draws it: one of its questions is the
  question shown, or, on the review, its headers are the strip.
- A session lookup that fails for a moment keeps the binding the last lookup gave and authorizes no
  key on that read, rather than giving the card another id that would start a new asking.
- The OmO test helper waits until its stand-in process runs its own code before the pane is read,
  so /proc shows that process's environment (the test that failed once in three runs).
- Removes an unreachable pane.get line from a prompt.test.ts fixture.
…and stop a move only on another call or strip

The omo reviewer's first pass on this round:
- omp's review binds its call by the review's rows, which omp draws whatever the strip's colours prove:
  its id no longer changes with them.
- A session lookup failure keeps the pane's last binding (that read authorizes no key) unless omp's
  session is not written yet; after three failures in a row the session reads as unproved. The map of
  last lookups keeps at most 64 panes.
- A move stops early only when another call is bound or the strip shows other labels; a frame drawn
  halfway (the same call and strip) is read again until the deadline, as before.
- pi's look before an answer key is held to the request's deadline again, as before this round.
The second review pass found that this round held pi's look (pi-ask's own check, which omo panes
run too) to the request's deadline by a race: an answer running past 4 s, such as a pi-questionnaire
answer typing its own text after several moves, had its next key refused. pi's look is awaited as
before; only the other agents' looks are held to the read's window.
@devswha

devswha commented Oct 31, 2026

Copy link
Copy Markdown
Owner Author

Round 1 after the merge pass's review, pushed as one push: 53654d7..a959f69 (4 commits).

Merge-pass findings

  • HIGH, a stale omp card drives another call: fixed. An omp card read under the pane's one open ask call names that call in its id, but only when the screen draws that call (one of its questions shown, or on the review, its headers as the review's rows). Before every key, on answers and moves alike, the call must still be the pane's one open ask call.
  • Read-to-send (prompt.ts): fixed. omp's foreground check and the call check are both bounded. The look before an answer key is held to the window of the read it follows (SETTLE_MS). pi's look is unchanged from main.
  • MEDIUM, the 4 s stall when the call changes mid-move: fixed. A move stops at once when another call is bound or the strip shows other labels. A frame drawn halfway (same call, same strip) is still read again until the deadline.
  • MEDIUM, the flaky OmO test: fixed. withOmoPane waits until its stand-in process runs its own code before /proc is read. The touched files ran green three times together.
  • MEDIUM, the changelog claimed review navigation: fixed. The changelog now matches the code: omp's review is not a step to move to and offers no moves.
  • LOW, the unreachable pane.get line: removed.

Reviews. Codex Terra 6.1 is out of quota until 2026-11-07, so Dori had the omo reviewer (Claude) stand in for it, twice.

  • Terra 6.1, one pass on the first commit, before it ran out:
    • Bind only a drawn call: fixed.
    • A transient lookup failure changed the id: fixed. The last binding is kept and that read authorizes no key; after three failures in a row the session reads as unproved.
    • Unbound cards and the partial-delivery result: accepted, see below.
  • omo reviewer pass 1, APPROVE:
    • The review id flipped with the tab colours: fixed, it binds by the review rows.
    • ConversationUnavailable blips changed the id: fixed.
    • A half-drawn frame ended a move: fixed, with a unit test in server/dialogs/step-navigate.test.ts.
    • The lookup map is capped at 64 panes.
    • The 1.5 s look test stays: deterministic, just slow.
  • omo reviewer pass 2, adversarial, on call identity and read-to-send, APPROVE:
    • Property A: no interleaving lets a key reach another call.
    • Property B, MEDIUM: this round had made pi's (and omo's) look race the request deadline, so a long answer could lose its next key. Fixed by restoring main's behaviour, with a test that is green 5 out of 5. On the old code the regression only showed sometimes, because it came down to two timers firing at the same moment.

Accepted, same as main:

  • An omp card whose call cannot be proven drawn still answers screen-only. That covers no session, zero or several open calls, and Markdown omp draws in ways the card can't match (a table, a preview, a cut description).
  • A provider that reused tool-call ids within one session would make a repeated ask look like the same call.
  • A key refused partway through an answer returns prompt_changed, as the per-key checks already did.

Validation

  • Every fix has a failing-first test that is red on the code before it, except the pi restoration noted above.
  • bun run check fast on a959f69: 10141 pass, 0 fail; typecheck and build ok.
  • Screenshots: phone 390 and desktop 1280, Codex dark, omp step strip and pi-ask Next, before and after Next. They are in the lane's report folder (~/.dori/reports/question-skip/2508/).

@devswha

devswha commented Nov 1, 2026

Copy link
Copy Markdown
Owner Author

Terra 6.1 pass at a959f69 (added because the earlier approval had no Codex review): changes, two MEDIUMs reproduced with in-process RPC stubs and a mkdtemp transcript (no herdr):

  1. server/prompt.ts:1461: the guard after the last screen read rechecks the foreground process and the open call but not the displayed step. If the terminal moves to review during that guard, Next still sends tab to the review screen, and the 409 comes after the key. Repro: REPRO screen=review checks=10 result={"status":409,"code":"prompt_changed"} sent=["tab"].
  2. server/omp-ask.ts:44: a clipped question's visible prefix is accepted as a match, so a stale screen of call A authorizes navigation on open call B when headers, options and the visible prefix are equal but the unseen suffix differs. Repro: clipped Which deployment should we choose for the production environment today… bound to call B ...today to delete everything?, result={"status":200} sent=["tab"].

Held for 0.4.23 as before; these need a round before it merges.

… step before Tab, and bind no cut question

Terra 6.1 on #2508 (two passes: the lane's and the merge pass's):
- Two of a call's questions drawn alike (other headers, the same text and options) shared one card id: a card
  read on the first could answer or move the second. The id now carries the question the strip proves is
  shown; without that proof the read authorizes no key.
- A move read the screen before its key's look, so the review could come up between them and still take Tab.
  The stop the key moves from is read again after the look, right before the key.
- A question omp cut at four rows could match another call that starts the same way. A cut question binds
  no call: moves need the whole question.
- The test that pi's look may end past the request's deadline moved the clock instead of using a 300 ms
  deadline, so it no longer depends on how fast the machine reads.
@devswha

devswha commented Nov 1, 2026

Copy link
Copy Markdown
Owner Author

Round 2 after the Terra 6.1 passes, pushed as one push: a959f69..9a6cf13 (one commit).

Findings, all fixed with failing-first tests:

  • Two questions drawn alike (lane's Terra, P1): two questions in one call with other headers but the same text and options shared one card id, so a card read on the first could answer or move the second. The id now carries the question the strip proves is shown. Without that proof, the read authorizes no key.
  • Step recheck before the key (merge pass's Terra, MEDIUM 1, prompt.ts): a move read the screen before its key's look, so the review could come up between them and still take Tab. The stop the key moves from is now read again after the look, right before the key.
  • Clipped question (merge pass's Terra, MEDIUM 2, omp-ask.ts): a question omp cut at four rows could bind another call that starts the same way. A cut question now binds no call, and moves need the whole question.
  • Deadline test timing (lane's Terra, P3): the test that pi's look may end past the request deadline used a real 300 ms deadline. It now moves the clock only while the look runs, so it doesn't depend on machine speed. It fails 3 of 3 runs on the earlier racing code and passes 3 of 3 on the fix.

Validation

  • bun run check fast passed on 9a6cf13: 10145 pass, 0 fail. It ran through the host's heavy-command gate, one job at a time, after the OOM restart.
  • The first attempt failed on the conventions test, which rejects fixed sleeps. I replaced the Bun.sleep in the new test with setImmediate before pushing.
  • Reviews: Codex Terra 6.1 (fast, xhigh), the lane's pass and the merge pass's pass, plus the omo reviewer's two passes from round 1.

@devswha

devswha commented Nov 1, 2026

Copy link
Copy Markdown
Owner Author

Delta review of round 2 (a959f69..9a6cf13) by the merge pass: Claude reviewer + Terra 6.1. Both round-1 MEDIUMs are fixed as described (the step is rechecked before Tab; a clipped question binds no call; alike questions are bound by position; a bound question that becomes clipped is refused). question-steps.test.ts passes 57/57 three times. Note: the injected-clock deadline test passes on both heads, so it fixes a flaky test, not production code.

Three MEDIUMs remain, reproduced by the merge pass on 9a6cf13 (HERDR_TEST_MODE=unit bun test, in-process RPC stubs, no herdr; the tests are kept by the merge pass and can be shared):

  1. server/prompt.ts:1737: the answer guard rechecks the call but not the tab. Switching to an alike second question while the guard is pending sends Enter there: ROUND2 alike screen=second result={"status":200} sent=["enter"].
  2. server/prompt.ts:1026 / :1469: after the last screen read an awaited process check runs; if the form is replaced during it, Tab still goes to the replacement and the 409 comes after the key: REPRO screen=review checks=11 result={"status":409,"code":"prompt_changed"} sent=["tab"].
  3. server/omp-ask.ts:83: a clipped question binds no call, but the card still restores its text from a prefix match, so with call A's clipped screen and call B's matching prefix in the transcript the card shows B's question while Enter reaches A: card="...today to delete everything?" result={"status":200} sent=["enter"].

LOW: pi/Claude tabs still send arrow keys without a recheck after the check before the key (server/dialogs/pi-ask-navigate.ts:42, untouched here); an omp question clipped past 4 rows offers no Back/Next, and a resize that clips or unclips it changes its id so an open card is refused once (fails safe).

Still for 0.4.23, after 0.4.22 ships and the owner's look.

…ly if it still shows what was tapped

The merge pass's Terra findings on #2508's round 2 were one class: a key sent after the screen changed under
a check. Every key a move or an answer sends now goes through one step (verifyThenSend): the look before the
key, then a read that settles (its last await is a read of the screen, parsed again with what the read
found, so a card the screen no longer draws authorizes nothing), then a check of the question the card was
tapped on (its id, which carries the call and, for questions drawn alike, the tab; the tab; the call; the
whole question), then the write, started with no await after that check. Moves (omo, Codex, Hermes, omp,
pi's and Claude's tabs) and every answer key up to and including the first that cannot be undone go
through it; the keys after that one (an editor's text) keep their path's own checks.

- The other of two questions drawn alike, switched in during the look before Enter, takes no Enter.
- A form replaced while the last read's own checks run takes no Tab or Enter.
- omp's Markdown is given only to a whole question, never by a prefix omp cut: a cut question stays as
  drawn. Two #1644 cases that restored a cut question now expect it as drawn.
- A Claude answer whose screen showed another question after its first move now sends no second move
  either (its test expected the second move).
@devswha

devswha commented Nov 1, 2026

Copy link
Copy Markdown
Owner Author

Final round, pushed as one push: 9a6cf13..856ff52 (one commit). This replaces the per-spot fixes with one verify step.

One step before every key: verifyThenSend in server/prompt.ts

  1. The look before the key (guardKey: pi-ask, omp in front, the omp call still open).
  2. A settling read (readKnownPrompt's new settle). Once all its other awaits are done, it reads the screen again in the format the card was read in. It then re-runs the same synchronous parse, with the call and session it found. If the card id or the current tab changed, the read authorizes no key.
  3. A synchronous check of the question the card was tapped on: its id (which carries the call and, for questions drawn alike, the tab), the tab, the call, and the whole question text. A menu read as an interaction (Claude's lists) or a pi dialog off its first row is checked on the screen text, as before.
  4. The write, started in the same continuation as that check, with no await between them.

What goes through it

  • Every move key (omo, Codex, Hermes, omp, pi's and Claude's tabs), through moveKeys.send.
  • Every answer key, up to and including the first one that cannot be undone, through sendStep.
  • Not covered: keys after that first irreversible one, such as an editor's typed text. The screen has already left the question by then, so those keys keep their path's own checks, as on main.

Your three findings, each a red test before this commit

  • Two questions drawn alike: the second one, switched in during the look before Enter, takes no Enter.
  • The review drawn during the last read's own checks: it takes no key, for Next and for an answer.
  • A cut question: omp's Markdown is now given only to a whole question, never by a prefix omp cut.

One existing test changed: in a Claude answer whose screen showed another question after its first ↓, the second ↓ is now refused as well. The old test expected two moves.

Validation

  • The four repro tests (from ~/.cache/mp4-state/repro2508/, written in this file's harness and asserting the correct behaviour) fail on 9a6cf13 and pass now, plus one regression test (a question bound whole, then shown cut).
  • bun run check fast passed on 856ff52: 10150 pass, 0 fail, run through the host's heavy-command gate.

@devswha

devswha commented Nov 1, 2026

Copy link
Copy Markdown
Owner Author

Delta review of round 3 (9a6cf13..856ff52), the last round, by the merge pass: Claude reviewer + Terra 6.1. No HIGH. The round's fixes hold: an alike second question and a form replaced during the final check both get 409 with no key; a clipped omp card keeps the terminal's own text. question-steps 62/62, prompt 404/404, omp-ask 12/12 (twice); 4 of the 5 new question-steps tests fail with the round's production code reverted; a pane read that never answers ends at the deadline with 409 and no key.

Per the last-round rule these remain as a follow-up (all need the screen to change within milliseconds of the key, and all reproduce on 9a6cf13 too):

  1. MEDIUM server/prompt.ts:1780-1784: sameQuestion checks the question, not the cursor; moving the terminal cursor from Red to Blue just before Enter answers Blue (200). Suggested: also compare the selected index.
  2. MEDIUM server/prompt.ts:1809-1817: no re-check after the first committed key in a Hermes multi-select; if the screen switches question after the first Space, the remaining keys answer the new one (200).
  3. MEDIUM (test) server/question-steps.test.ts:492 passes with the round reverted, so it does not cover this change.
    LOW: the window between the last read and herdr acting on the key cannot be closed from Ovis; a Claude ←/→ frame caught mid-redraw refuses the move instead of retrying (never a wrong key); the omp re-check reuses the call lookup from the same read.

Approved for 0.4.23: merges after 0.4.22 ships and the owner has seen ~/.dori/reports/question-skip/2508/. Repro tests are kept by the merge pass.

@devswha

devswha commented Nov 1, 2026

Copy link
Copy Markdown
Owner Author

Merge pass: round 3 (856ff52) breaks two contract tests that pass on main (94b34b4); reproduced on this PC in an isolated session (bun run check run bun test ./server/<file> -t ...), and they fail the same way in CI (Integration tests):

  1. server/prompt.contract.test.ts "answers to Claude's unnumbered menus > takes an answer to a fallback card whose screen only ticked since it was read": Expected: 200, Received: 409. The new read before every key now refuses a fallback card whose screen only ticked (a timer or spinner changed) since it was read; main still answers it.
  2. server/omp.contract.test.ts "shows an omp Ask card in the open call's Markdown, answered by the keys of the menu on screen (OMP Ask cards: the card disappears under omp's status row, and Markdown in the question is flattened #1644)": the card id changed (Expected: "a67a34cbdba3", Received: "81c32decc336"), so the omp Ask card's id is no longer stable between reads.
    bun run check fast does not run contract tests; please run these two files (and the rest of server/*.contract.test.ts touching prompts) through bun run check run before the next push.

…a fallback card by what it asks

Two contract tests that pass on main failed after the one verify step:
- A fallback card whose screen only ticked since it was read (a spinner, a timer, a token count) was
  refused: its key's verifying read looked for a reader's card, which a fallback screen has none of.
  A fallback card is now checked by the card the screen still parses to (what it asks and offers).
- An omp card's id changed between reads of the same card: the id carried the open ask call, so a read
  that proved no call gave another id and started another asking. The id is the screen's again; the
  call is kept with the pane's asking, and only a read under another call starts a new one, so a card
  read under one call still answers no other call drawn the same.
- The verifying read draws the card as the answer's first read did (omp's own Markdown), so the
  question compares alike.
@devswha

devswha commented Nov 1, 2026

Copy link
Copy Markdown
Owner Author

Contract-test fix round, pushed as one push: 856ff52..1a5ff21 (one commit).

  • prompt.contract, "a fallback card whose screen only ticked since it was read": the key's verifying read looked for a reader's card, and a fallback screen has none, so the answer got 409. A fallback card is now checked by the card the screen still parses to: what it asks and offers. A spinner, timer or token count that ticks no longer refuses it.
  • omp.contract OMP Ask cards: the card disappears under omp's status row, and Markdown in the question is flattened #1644, "omp Ask card": the card's id carried the open ask call, so a read that proved no call (the call answered, then restored) gave another id and started another asking.
    • The id is the screen's again.
    • The call is now kept with the pane's asking. Only a read under another call starts a new asking, so a card read under one call still answers no other call drawn the same; that test stays green.
    • The verifying read also draws the card in the same Markdown mode as the answer's first read, so the questions compare alike.

Validation (heavy-command gate, one job at a time)

  • bun run check run bun test server/prompt.contract.test.ts server/omp.contract.test.ts: 19 pass, 0 fail.
  • Touched unit suites: 890 pass, 0 fail, including the three race repros and the pending-guard repro.
  • bun run check fast on 1a5ff21: 10150 pass, 0 fail.

@devswha

devswha commented Nov 1, 2026

Copy link
Copy Markdown
Owner Author

Merging 1a5ff21 into 0.4.22 (owner: most work goes into 0.4.22): pi-ask and omp question cards move forward to unanswered questions (Next, Back and steps), with every key bound to the form the user tapped.

Strict review by the merge pass over four rounds (Claude reviewer + Terra 6.1 each round). Fixed along the way and re-checked on this head with Terra's own repros (mocked RPC, no herdr): the step is re-read right before every key; a form replaced during the final check, or an alike question switched in during the answer guard, gets 409 with no key; a clipped question binds no call and its card shows the terminal's own text; alike questions are bound by position. The last round's contract regressions are fixed: prompt.contract + omp.contract 19/0 (a fallback card whose screen only ticked is answered again; the omp card id is stable). question-steps 62/62, prompt 404/404, omp-ask 12/12.

MEDIUM, follow-up (last-round rule): prompt.ts checks the question but not the cursor right before Enter, so a terminal cursor moved from Red to Blue at that instant answers Blue; in a Hermes multi-select the keys after the first toggle are not re-checked; question-steps.test.ts:492 passes with its round reverted. LOW: the window between the last read and herdr acting on a key cannot be closed from Ovis; pi/Claude ←/→ moves have the same gap (pi-ask-navigate.ts); an omp question clipped past 4 rows offers no Back/Next.

@devswha
devswha merged commit b2382f2 into main Nov 1, 2026
20 checks passed
@devswha
devswha deleted the fix/question-card-forward-steps branch November 1, 2026 05:06
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