fix: preserve literal args, reject inconsistent history pages, load closed-thread neighbors - #220
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
|
Codex review: needs maintainer review before merge. Reviewed September 28, 2026, 6:16 AM ET / 10:16 UTC. ClawSweeper reviewWhat this changesThe branch preserves literal CLI arguments after 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 Review scores
Verification
How this fits togetherGitcrawl 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]
Before mergeNone. Agent review detailsSecurityNone. Review metricsNone. Technical reviewBest 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. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
|
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--normalizeCommandArgsreorders flags ahead of positionals, and it also moved flag-looking text that appears after the end-of-options marker. Sogitcrawl search … -- --literal-textfailed or changed options. Arguments after--now stay positional.Tests:
TestNormalizeCommandArgsPreservesEndOfOptions,TestGHSearchLiteralFlagQuery.fix(sync): reject inconsistent GraphQL continuation countsHistory pagination keeps the first page's connection map, so the final completeness check compares the first page's
totalCountwith the number of accumulated children. Suppose one comment before the cursor is deleted between pages. The continuation reports a smallertotalCount, 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 sametotalCountas 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 threadBoth 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 fullgo test ./..., focused-raceruns over the touched packages, and the docs build and tests. No CHANGELOG edit here; release notes are written at release preparation.