glitter: whole-binary pty gate, pty flake fix, and doc drift - #356
Merged
Merged
Conversation
…nd glitter README claimed `rt git commit` has a live diff preview; it has never had one. `rt git reset soft|hard` both target HEAD, which reads as "undo my commit" to anyone who has not used them. `rt git undo` and `rt glitter` both shipped without a README entry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Drives the compiled rt spawning the real rt-ui, asserting git state after each step rather than rendered text. Screen text appears only in waits, and every wait keys on a state transition the board paints after the driver returns from git, so there are no fixed sleeps to tune. Verified it catches regressions: changing the space keybinding's payload from toggle-file to toggle-hunk fails two of the four tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Terminal setup (alt screen, cursor hide, the Kitty keyboard push) reaches the pty before any cell is drawn, so the old predicate released while the screen was still blank. Measured on an idle machine it fired 1.4-4.2ms early; the keys loop's 30ms delay absorbed that locally but not under CI load, where a key landed pre-paint, the program quit, and the assertion ran against an empty screen. That is the TestTextInputDrawsOneChevronPrompt failure seen on rt#354. The predicate now replays the buffer and requires a non-blank screen. The no-paint deadline goes to 10s since it is now waiting on a real paint. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Understand this PR’s impact Explore downstream dependencies and potential security impact with Blast Radius. Warning Review limit reachedNext included review available in 18 minutes. View limit detailsLimit details: You’ve used all 2 included reviews currently available. Your 79 included PR review attempts over the past 7 days set your current allowance at 2 reviews per hour. Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. Review configuration: ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (6)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (7)
Included review availability: 0 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 2 reviews per hour. 📝 WalkthroughWalkthroughThe change adds an isolated Git fixture and PTY tests for ChangesGlitter PTY coverage
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Other Sequence Diagram(s)sequenceDiagram
participant glitter-pty.test as glitter-pty.test
participant TermwrightSession
participant rt-glitter as rt glitter
participant GitRepository as Git repository
glitter-pty.test->>TermwrightSession: Send Kitty keyboard and raw PTY input
TermwrightSession->>rt-glitter: Forward terminal bytes
rt-glitter->>GitRepository: Read and update repository state
rt-glitter-->>TermwrightSession: Draw updated board
TermwrightSession-->>glitter-pty.test: Return painted screen state
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 22.22% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 5 files. (2 skipped: 2 unsupported.) ✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The gate needs Go and an rt-ui build the other e2e tests never touch, which put ~60s of setup on every PR. It moves to e2e/pty/ and its own job, gated on a changed-path filter, so the cost lands only where the board can break and an inapplicable run reads as a skipped check rather than a silent pass. The filter is every input the board is built from, not just ui/: the driver, the command, and git-core are TypeScript, and both bugs this gate was written after were TypeScript with no Go diff. Checked against real ranges: rt#354 (TS-only glitter fix) runs it, the RT-192 docs commit does not. Co-Authored-By: Claude Opus 5 (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.
What
RT-220 — a whole-binary gate for
rt glitter. Everything under it was previously gated with something faked on one side: the Go render tests script the driver, the compose test scripts the session.e2e/tests/glitter-pty.test.tsruns the real compiledrtspawning the realrt-uiover a pty and asserts git state after each step, never rendered text.It covers four paths: the board opens on the tree git actually has;
spaceunchecks the cursor's file and the commit omits it while leaving it modified; agit adddone behind the board's back is discarded by the commit-time index rebuild (the GHD model, asserted rather than documented); anduundoes the commit and keeps the work.Screen text appears only in waits, and every wait keys on a state transition the board paints after the driver returns from git (
Commit N files), so there are no sleeps to tune. Five consecutive local runs, 20/20 assertions.I verified it actually catches regressions rather than assuming: changing the
spacekeybinding's payload fromtoggle-filetotoggle-hunkfails two of the four tests.Reused
e2e/interactive.ts(termwright), which was already in the repo and already installed by the e2e workflow, rather than adding a second pty harness. It gained one method,raw(bytes), for key encodingspress/ctrlcannot express — the board negotiates the Kitty keyboard protocol, so ctrl+enter is a CSI-u sequence.The pty flake, fixed at the cause.
TestTextInputDrawsOneChevronPromptfailed CI on rt#354 with an empty painted screen.runPTY's paint predicate returned true on the first pty byte, but terminal setup (alt screen, cursor hide, Kitty keyboard push) arrives before any cell is drawn. Instrumented on an idle machine, it released 1.4–4.2ms early; the keys loop's 30ms delay absorbed that locally but not under CI load, where a key landed pre-paint, the program quit, and the assertion ran against a blank screen. The predicate now replays the buffer and requires a non-blank screen.RT-192 — doc drift. The README claimed
rt git commithas a "live diff preview" thatcommands/commit.tsexplicitly denies.rt git reset soft|hardboth target HEAD, which reads as "undo my commit". Beyond what the ticket listed,rt git undoandrt glitterhad both shipped with no README entry at all, so the intended "point atrt git undo" fix had nowhere to point. Added both, and put thert git undosteer into theresetcommand descriptions so it reaches--helpand the picker too.CI. The e2e job now sets up Go, since the gate drives the real rt-ui. The gate lives in the existing e2e job, which is already required.
Verification
Run from the worktree:
go vet ./...clean;go test -count=1 ./...clean; prompt package green over-count=10.bunx tsc --noEmitclean;bash scripts/repo-purity.shok.bun run test:e2e: 129 pass (4 new), 1 fail —user plugins > rt plugin new, which fails identically onmain(a broken localmisenode shim in the test's sanitized env, unrelated to this branch).Closes RT-192, closes RT-220.
🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
rt git undo.rt glitterterminal board, including staging, diffs, history, branches, commits, and interactive requirements.Tests