Skip to content

fix: preserve literal args, reject inconsistent history pages, load closed-thread neighbors - #220

Merged
steipete merged 3 commits into
mainfrom
fix/bug-sweep-2026-09-28
Sep 28, 2026
Merged

steipete merged 3 commits into
mainfrom
fix/bug-sweep-2026-09-28

Conversation

@steipete

Copy link
Copy Markdown
Contributor

Three reproducible defects from a bounded bug sweep of main. Each is fixed in its own commit, and each regression test fails before its fix.

fix(cli): preserve arguments after --

normalizeCommandArgs reorders flags ahead of positionals, and it also moved flag-looking text that appears after the end-of-options marker. So gitcrawl search … -- --literal-text failed or changed options. Arguments after -- now stay positional.
Tests: TestNormalizeCommandArgsPreservesEndOfOptions, TestGHSearchLiteralFlagQuery.

fix(sync): reject inconsistent GraphQL continuation counts

History pagination keeps the first page's connection map, so the final completeness check compares the first page's totalCount with the number of accumulated children. Suppose one comment before the cursor is deleted between pages. The continuation reports a smaller totalCount, but the child count can still equal the first page's total, so a snapshot containing the deleted comment was accepted as complete. Every continuation page must now report the same totalCount as the first. Comments appended mid-pagination were already rejected by the existing check, so this adds no new failure class for ordinary activity.
Test: TestGraphQLHistoryRejectsChangedContinuationCount (increased, decreased and missing counts).

fix(tui): load neighbors for a selected closed thread

Both target-vector lookups filtered out closed threads. Selecting a closed thread and loading neighbors therefore reported a missing embedding even though one was stored. The selected thread may now be closed; neighbor candidates keep their existing open-thread filtering.
Test: TestTUILoadNeighborsForClosedSelection (configured and fallback model).

Gates: make fmt (clean), go vet ./..., the full go test ./..., focused -race runs over the touched packages, and the docs build and tests. No CHANGELOG edit here; release notes are written at release preparation.

@clawsweeper

clawsweeper Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Sep 28, 2026
@clawsweeper

clawsweeper Bot commented Sep 28, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed September 28, 2026, 6:16 AM ET / 10:16 UTC.

ClawSweeper review

What this changes

The branch preserves literal CLI arguments after --, rejects GraphQL history pages whose counts change during pagination, and lets a selected closed thread seed terminal-browser neighbor search.

Merge readiness

✅ Ready for maintainer review

Keep open. Current main retains the three reported failure paths, and this member-authored PR provides focused repairs with regression coverage. I found no concrete introduced defect.

Priority: P2
Reviewed head: dc43e15481ac189891969d9c5ef3d6910401f047

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) The three repairs are focused and documented with relevant regression coverage, with no concrete patch defect found.
Proof confidence 🌊 off-meta tidepool Not applicable: The member-authored PR is exempt from contributor runtime proof; its changed production paths are CLI parsing, GraphQL page acceptance, and local neighbor lookup, with focused regression tests but no independently observed live run. No stored-data contract changes.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: The member-authored PR is exempt from contributor runtime proof; its changed production paths are CLI parsing, GraphQL page acceptance, and local neighbor lookup, with focused regression tests but no independently observed live run. No stored-data contract changes.
Evidence reviewed 6 items CLI behavior on main: The current-main normalizer moves flag-looking arguments without stopping at the end-of-options marker.
CLI repair and regression coverage: The introduced marker guard preserves following positional text; the branch also adds a full App search test using a dash-prefixed query.
History pagination repair: Main checks the accumulated children against the first page's total only at completion. The branch additionally checks every continuation page's total before accepting its nodes.
Findings None None.
Security None None.

How this fits together

Gitcrawl accepts search arguments and GitHub conversation pages, then stores searchable data locally. Its terminal browser uses saved thread embeddings to find related open threads.

flowchart LR
A[CLI query arguments] --> B[Option parser]
B --> C[Local search results]
D[GitHub history pages] --> E[Count validation]
E --> F[Local archive]
G[Selected thread embedding] --> H[Open neighbor results]
Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

None.

Technical review

Best possible solution:

Retain focused regression coverage for each repaired path so later parser, sync, and terminal-browser changes preserve these behaviors.

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

Yes, current-main source identifies each failing path and the branch adds focused regression cases. This read-only review did not execute them.

Is this the best way to solve the issue?

Yes. The patch changes the argument boundary, validates each continuation count, and broadens only the selected-thread lookup while keeping neighbor candidates filtered.

AGENTS.md: not found in the target repository.

Codex review notes: model internal, reasoning high; reviewed against 290d943b1c0c.

Labels

Label changes:

  • add P2: The PR repairs three bounded search, sync, and terminal-browser failures without evidence of an urgent widespread outage.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The member-authored PR is exempt from contributor runtime proof; its changed production paths are CLI parsing, GraphQL page acceptance, and local neighbor lookup, with focused regression tests but no independently observed live run. No stored-data contract changes.

Label justifications:

  • P2: The PR repairs three bounded search, sync, and terminal-browser failures without evidence of an urgent widespread outage.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The member-authored PR is exempt from contributor runtime proof; its changed production paths are CLI parsing, GraphQL page acceptance, and local neighbor lookup, with focused regression tests but no independently observed live run. No stored-data contract changes.

Evidence

What I checked:

  • CLI behavior on main: The current-main normalizer moves flag-looking arguments without stopping at the end-of-options marker. (internal/cli/args.go:8, 290d943b1c0c)
  • CLI repair and regression coverage: The introduced marker guard preserves following positional text; the branch also adds a full App search test using a dash-prefixed query. (internal/cli/args.go:8, dc43e15481ac)
  • History pagination repair: Main checks the accumulated children against the first page's total only at completion. The branch additionally checks every continuation page's total before accepting its nodes. (internal/github/history.go:277, dc43e15481ac)
  • Closed-thread lookup scope: The branch includes closed threads for both selected-target lookups while leaving the candidate-vector query's open-thread filter in place. (internal/cli/tui_neighbors.go:106, dc43e15481ac)
  • Feature history: Merged history identifies the GraphQL history feature, terminal-browser refactor, cached search feature, and earlier argument-normalization work. (internal/github/history.go:214, bd9143104d03)
  • Release comparison: The three production files are unchanged between v0.12.0 and current main, so the branch's repairs are not already present in that release or main. (290d943b1c0c)

Likely related people:

  • Hannes Rudolph: Raw commit bd91431 adds internal/github/history.go:214 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: bd9143104d03; files: internal/github/history.go)
  • Peter Steinberger: Raw commit dc565b4 adds internal/cli/tui_neighbors.go:96 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: dc565b4fc007; files: internal/cli/tui_neighbors.go)
  • Vincent Koc: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

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 merged commit 56ece89 into main Sep 28, 2026
18 checks passed
@steipete
steipete deleted the fix/bug-sweep-2026-09-28 branch September 28, 2026 11:23
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: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant