gui: Show every mismatch when a card is read back - #109
BenWestgate wants to merge 5 commits into
Conversation
The read-back page named only the first wrong group and needed a click on "Confirm card". After a mismatch it now redraws what was typed, with every wrong group highlighted, and Enter confirms once the entry is complete. The original card stays hidden; clearing the entry also drops the redrawn copy. This ports the current GUI line's behaviour onto the Bails pin without the newer GUI's other changes, so restore keeps its recorded-fingerprint check. Validation: 989 tests pass normally and under python -O with GTK 4 under Xvfb, including two new read-back tests that fail on the old page; Ruff, format and mypy are clean; the GUI stays under its 2000-line budget. Claude-Session: https://claude.ai/code/session_013gZvwvuocM7a7Ut4kiBFHw
|
nit: It should also lock from editing the groups that are already correct. |
After a mismatch, only the highlighted groups can still be edited. An edit that reaches a group that matched is undone. An edit inside an open group is cut, or padded with "?", back to that group's size, so the card keeps its length and no later group shifts. Clearing the field unlocks it. Claude-Session: https://claude.ai/code/session_018HrKoywz6PT9c91jvKou7Q
Join five function signatures and two calls that fit on one line, so the GUI is 1996 lines, under its 2000-line budget, after the matched-group lock. Only formatting changes: the module's AST is identical. Validation: 992 tests pass, normally and under -O; ruff, format and mypy are clean. Claude-Session: https://claude.ai/code/session_018HrKoywz6PT9c91jvKou7Q
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1027e599cd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| start = next((i for i in range(same) if old[i] != new[i]), same) | ||
| tail = next((i for i in range(same - start) if old[-1 - i] != new[-1 - i]), same - start) |
There was a problem hiding this comment.
When an editable group contains adjacent identical symbols, this first-difference calculation cannot determine which copy the operator deleted and ignores the GTK cursor position already captured by _reformat. For example, deleting the first B from ABBC produces ABC? with the cursor after the remaining B, so typing X yields ABXC rather than the intended AXBC. This breaks the promised in-place correction for valid cards containing repeated characters; use the actual edit/cursor location when padding the deletion.
Useful? React with 👍 / 👎.
In a run of repeated symbols the first difference between the old and new text does not say which copy was deleted. Place the edit at the cursor GTK left, so deleting the first P of QPPZ leaves the cursor after Q and the next keystroke lands there. Claude-Session: https://claude.ai/code/session_018HrKoywz6PT9c91jvKou7Q
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d17822a550
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| self._dropped, self._locked = "", frozenset() | ||
| self.set_text(PREFIX) |
There was a problem hiding this comment.
When a read-back page is left or confirmation succeeds, clear() replaces only the GTK text and unlocks the entry, but _shown still references the complete recovery card last assigned by _reformat. set_text(PREFIX) merely schedules another idle reformat, so the card remains reachable after the page cleanup—and indefinitely if that idle callback does not run—contrary to the recovery-text clearing guarantee in docs/security/model.md. Reset _shown to PREFIX directly in clear() as part of the synchronous cleanup.
AGENTS.md reference: AGENTS.md:L78-L82
Useful? React with 👍 / 👎.
clear() replaced the field's text but left the last complete card in _shown until the next idle reformat ran. Reset it in clear() itself, so no copy of the recovery text outlives leaving the page. Claude-Session: https://claude.ai/code/session_018HrKoywz6PT9c91jvKou7Q
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e4f96aea5c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| while start and len(new) - tail > cursor and new[-1 - tail] == old[-1 - tail]: | ||
| start, tail = start - 1, tail + 1 |
There was a problem hiding this comment.
When a selection is replaced with text sharing a suffix with the old group, this adjustment only handles inferred edit boundaries to the right of GTK's cursor. For example, replacing a selected editable QQQQ group with WQQQ leaves the text correct but infers only W as typed and moves the cursor after W instead of after the replacement; subsequent typing then edits the wrong position. Fresh evidence beyond the earlier deletion case is that the current fix still fails for multi-character replacements whose trailing symbols match the old text.
Useful? React with 👍 / 👎.
This branch has not been deployed
2 participants
Requested by Ben · project thread
What
On the card read-back page (Bails's pinned GUI,
1938b60):?, so the card keeps its length and later groups never shift. Clearing the field unlocks it.Why
Ben tested this in Bails and found the re-entry help far weaker than the CLI's, and that Enter didn't submit. The current GUI line (
gui-reviewability-v1) already does the highlighting and Enter. Repinning Bails to it would drop restore's recorded-fingerprint check, so this ports those behaviours onto the pin. Ben then asked for the matched groups to be locked.Notes
entry.pygainsCodex32Entry.lock();pages.pycalls it after a mismatch.1027e59joins five wrapped signatures and two calls onto single lines. It changes formatting only (the module's AST is identical). The GUI is now 1996 lines.bails-recovery-1938b60) and repin Bails'sinstall-codex32andopen-codex32to it.Validation
python -O.tests/test_gui_read_back.pycovers Enter, mismatch highlighting, clearing, locked groups, open-group editing and unlocking. The new tests fail on the old page and pass on the new one. They skip without GTK or a display, as on CI.src/codex32,src/codex32_gui) are clean.AI-written (Claude); requires responsible-human review per
docs/developer/AI_POLICY.md.🤖 Generated with Claude Code
https://claude.ai/code/session_018HrKoywz6PT9c91jvKou7Q