Skip to content

fix(tui): filter typing, rune-safe backspace, refresh stacking (v0.14.6 prep) - #82

Merged
steipete merged 1 commit into
mainfrom
steipete/tui-filter-fixes
Aug 7, 2026
Merged

fix(tui): filter typing, rune-safe backspace, refresh stacking (v0.14.6 prep)#82
steipete merged 1 commit into
mainfrom
steipete/tui-filter-fixes

Conversation

@steipete

@steipete steipete commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Release preparation for v0.14.6: three TUI fixes hit downstream crawl apps that wrap crawlkit/tui.

  • Filter mode: typing any word containing a certain letter used to quit the app because the quit case matched it before the rune-append branch. That letter now appends to the query; ctrl+c and ctrl+d still quit while filtering.
  • Filter backspace removed one byte instead of one rune, corrupting CJK/emoji queries into invalid UTF-8. It now strips exactly the last rune.
  • startRefresh never checked m.refreshing, so slow loads stacked overlapping refresh goroutines from the auto-refresh tick. It now returns nil while a refresh is in flight and reports "Refresh already in progress" on manual refresh.

Regression tests cover all three; the stale test asserting the old quit behavior was updated. Changelog section v0.14.6 is dated for release.

Proof: GOWORK=off go vet ./..., GOWORK=off go test -count=1 ./..., make check, actionlint - all green. Codex autoreview clean.

…tacking

- typing q in filter mode now appends to the query; ctrl+c/ctrl+d still quit
- backspace removes one rune instead of one byte, keeping CJK/emoji queries valid UTF-8
- startRefresh guards m.refreshing so slow refreshes no longer stack goroutines
- date the v0.14.6 changelog section for release
@steipete steipete closed this Aug 6, 2026
@steipete steipete reopened this Aug 6, 2026
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. labels Aug 6, 2026
@clawsweeper

clawsweeper Bot commented Aug 6, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed August 6, 2026, 7:43 PM ET / 23:43 UTC.

ClawSweeper review

What this changes

The PR fixes filter-mode q input, rune-safe filter backspace, and overlapping refresh starts in the shared terminal archive browser, with regression tests and release-preparation notes.

Merge readiness

Blocked until real behavior proof from a real setup is added - 3 items remain

Keep open pending real terminal behavior proof. The source patch correctly repairs three current shared-TUI defects and has focused regression coverage.

Priority: P2
Reviewed head: 167b682f8343801a347cfe7a825171ae6645747c

Review scores

Measure Result What it means
Overall readiness 🦪 silver shellfish (2/6) The source patch is focused and regression-tested, but real interactive terminal proof remains a merge gate.
Proof confidence 🦪 silver shellfish (2/6) Needs real behavior proof before merge: The PR reports automated checks and unit tests but supplies no after-fix terminal-session evidence; add redacted live output, a screenshot, or a recording before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Needs proof Needs real behavior proof before merge: The PR reports automated checks and unit tests but supplies no after-fix terminal-session evidence; add redacted live output, a screenshot, or a recording before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed 3 items Current-main defect: Current main treats q as quit while filtering, truncates filter text by byte, and starts refreshes without checking whether one is already running.
Release preparation is supported: Release documentation explicitly instructs a release-preparation PR to date the versioned changelog before the release workflow is dispatched.
Structured-review infrastructure: The required autoreview helper was invoked for the PR commit but could not start because this sandbox has no usable temporary directory; the checkout remained clean.
Findings None None.
Security None None.

How this fits together

The shared TUI receives keyboard and refresh-timer events from downstream crawl applications, maintains filter and reload state for archive rows, and renders the interactive terminal browser.

flowchart LR
A[Keyboard and refresh timer] --> B[Shared terminal browser]
B --> C[Filter query state]
B --> D[Refresh in-flight check]
C --> E[Filtered archive rows]
D --> F[Archive reload]
F --> E
Loading

Before merge

  • Add real behavior proof - Needs real behavior proof before merge: The PR reports automated checks and unit tests but supplies no after-fix terminal-session evidence; add redacted live output, a screenshot, or a recording before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
  • Resolve merge risk (P1) - No after-fix interactive terminal recording, screenshot, or live transcript is provided; automated tests alone do not meet the real-behavior proof gate.
  • Complete next step (P2) - There is no discrete code repair remaining; contributor-provided real terminal proof is the outstanding human-only merge gate.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Patch scope 3 files affected; production +10/-2, tests +60/-3 The implementation remains tightly scoped to shared TUI input and refresh behavior.

Merge-risk options

Maintainer options:

  1. Decide the mitigation before merge
    Retain the focused shared-TUI repair and add redacted terminal evidence showing q filtering, Unicode backspace, and the already-in-progress refresh result.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Technical review

Best possible solution:

Retain the focused shared-TUI repair and add redacted terminal evidence showing q filtering, Unicode backspace, and the already-in-progress refresh result.

Do we have a high-confidence way to reproduce the issue?

Yes—source provides a high-confidence path: enter filter mode and type q, backspace a multi-byte query, or issue a refresh tick while a slow refresh is pending.

Is this the best way to solve the issue?

Yes—the changes are narrow, preserve the generic shared TUI boundary, and use the repository’s documented release-preparation pattern for the changelog date.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 171b8328fc36.

Labels

Label changes:

  • add P2: The PR repairs concrete shared terminal-browser correctness defects without evidence of data loss, security impact, or an urgent outage.
  • add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR reports automated checks and unit tests but supplies no after-fix terminal-session evidence; add redacted live output, a screenshot, or a recording before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Label justifications:

  • P2: The PR repairs concrete shared terminal-browser correctness defects without evidence of data loss, security impact, or an urgent outage.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR reports automated checks and unit tests but supplies no after-fix terminal-session evidence; add redacted live output, a screenshot, or a recording before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

What I checked:

  • Current-main defect: Current main treats q as quit while filtering, truncates filter text by byte, and starts refreshes without checking whether one is already running. (tui/tui.go:841, 171b8328fc36)
  • Release preparation is supported: Release documentation explicitly instructs a release-preparation PR to date the versioned changelog before the release workflow is dispatched. (docs/publishing.md:39, 171b8328fc36)
  • Structured-review infrastructure: The required autoreview helper was invoked for the PR commit but could not start because this sandbox has no usable temporary directory; the checkout remained clean. (167b682f8343)

Likely related people:

  • Peter Steinberger: Available blame attributes the affected current TUI lines to the partial-history boundary authored by Peter Steinberger; deeper history was unavailable locally. (role: current source-attribution boundary; confidence: low; commits: 4b80ce12b866; files: tui/tui.go)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Post redacted terminal evidence showing q filtering, Unicode backspace, and a manual refresh while a refresh is already in flight.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@steipete steipete closed this Aug 7, 2026
@steipete steipete reopened this Aug 7, 2026
@steipete
steipete merged commit 3c3f835 into main Aug 7, 2026
23 checks passed
@steipete
steipete deleted the steipete/tui-filter-fixes branch August 7, 2026 00:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant