Repository navigation
neighbors cannot tell merged rows from closed ones or filter by time, so a superseded-PR check takes ~18 s per PR #238
Description
Activity
- added
on Oct 10, 2026 Codex review: this still needs some work.
Summary
This issue remains necessary: current main lacks the requested filters and result metadata, and an open same-author PR explicitly owns the implementation.Reproducibility: Current source establishes the absent filters and metadata and the loading-before-limiting path; the contributor supplies CLI evidence, but this review ran no target code.
Root-cause cluster
Relationship:canonical
Canonical: #238
Summary: This issue tracks the missing neighbor-selection capability, and the linked open PR proposes its implementation.Members:
fixed_by_candidate: feat(neighbors): filter by state and merge time before ranking #239 - Its body explicitly closes this issue and proposes the requested filters and output metadata; it remains unmerged.
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.
Maintainer decision needed
- Question: Should neighbors expose all three proposed time filters, additive thread metadata, and the state_as_of freshness contract?
- Recommendation: Approve the proposed surface: Review feat(neighbors): filter by state and merge time before ranking #239 with preserved defaults and explicit reopening and sweep-coverage limitations.
- Why: The report demonstrates practical value, but the new public flags and historical-state semantics require owner judgment, and the inspected discussion records no human decision.
Next step
Review #239 and settle its filter and output contract before resolving this issue.Review details
Best possible solution:
Provide approved SQL filters before vector decoding and ranking, with additive metadata and clearly documented archive freshness limits.
Do we have a high-confidence way to reproduce the issue?
Current source establishes the absent filters and metadata and the loading-before-limiting path; the contributor supplies CLI evidence, but this review ran no target code.
Is this the best way to solve the issue?
Filtering within the existing SQL selection directly addresses the workload without a storage migration; the linked PR is the appropriate place to settle public semantics and compatibility.
AGENTS.md: found but not applied because it conflicted with ClawSweeper's review contract.
Remaining risk / open question:
- Historical open-state selection relies on current archive values, so reopening and incomplete closed-thread sweep coverage limit its accuracy.
- The reported timings and exact SQL-variable failure threshold were not independently reproduced.
Codex review notes: model internal, reasoning medium; reviewed against bd30790d7735.
Label changes
Label changes:
No label changes.
Label justifications:
P2: A useful large-archive search improvement with limited operational blast radius and an existing implementation PR.impact:ux-friction: Finding later-merged candidates requires large neighbor retrieval and a separate thread-dump join.
Evidence reviewed
What I checked:
- Verified default branch: GitHub identifies main as the default branch; its current SHA matches the inspected checkout. (bd30790d7735)
- Requested command surface remains absent: The command registers number, limit, threshold, include-closed and json; neighbor rows contain thread_id, number, kind, title and score, without state or merge timestamps. (
internal/cli/neighbors.go:18, bd30790d7735) - Selection loads vectors before ranking: ThreadVectorQuery has no time predicates, and ListThreadVectorsFiltered decodes every selected vector; ThreadsByIDs constructs one SQL placeholder per returned ID. (
internal/store/vectors.go:63, bd30790d7735) - Existing performance work does not supply filters: Exact search still scores selected vectors before TopK; the merged perf(vector): prepare embeddings once and add portable SIMD scoring #218 improves scoring, while fix: preserve literal args, reject inconsistent history pages, load closed-thread neighbors #220 fixes neighbor loading for a closed TUI selection. (
internal/vector/exact.go:62, bd30790d7735) - Documentation and existing coverage agree: Neighbor documentation lists the existing flags; CLI coverage checks similarity results, and store coverage checks embedding variants and current open-state selection. (
docs/clustering.md:118, bd30790d7735) - Open implementation owns the work: feat(neighbors): filter by state and merge time before ranking #239 remains open and unmerged, explicitly closes this issue, and supplies baseline and changed CLI output on an existing archive; its discussion contains bot reviews but no recorded human product decision. (44f46b6549cb)
How this review workflow works
- 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-reviewcomments, 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-reviewor@clawsweeper re-runon an open PR or issue to request a fresh review only. - Maintainers can also comment
@clawsweeper reviewto 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 explainto ask for more context, or@clawsweeper stopto stop active automation.
Reviewed October 10, 2026, 6:27 PM ET / 22:27 UTC.
Summary
A common cleanup question on a large repository is "was this open PR already landed by a later one?". gitcrawl's
neighborsis the natural tool: the answer is among the similar pull requests merged after the open PR was created. Todayneighborscannot tell a merged row from a closed one and cannot filter by time, and its rows carry no state. The only route is--include-closedwith a very large--limit, plus a separatethreads --include-closed --jsondump to look up each row's merge time. That scores every stored vector on every call: about 18 s per PR on a warm cache, whatever--limitis, and rows that rank below the limit are lost.How often the question comes up
On openclaw/openclaw, 2026-09-01 to 2026-10-09, external pull requests (from authors outside the maintainer team, bots excluded) closed without merging:
The delay sits in the cases where the later pull request did not mention the earlier one, which is what a similarity search with state and time can surface.
What happens today
gitcrawl main bd30790, on a read-only copy of an openclaw/openclaw archive (165,815 threads, 3,306 open pull requests, last complete sync 2026-10-10T09:06:46Z), embeddings
qwen3-embedding:0.6b(1024 dimensions,title_originalbasis):neighbors: 18.3 s median per PR with a warm page cache (10 open PRs; 18.0–18.5 s at--limit1,000, 5,000 or 32,765), 38–39 s cold, about 3.3 GB RSS. It decodes and scores all 165,815 stored vectors each time.threadsdump (992 MB of JSON) adds 5.5 s warm or 32 s cold.--limitabove 32,765 fails withtoo many SQL variables, so a candidate ranked lower can never be returned.Proposal
Add time filters to
neighbors, applied in SQL before vectors are scored and before the limit:--merged-after <time>: pull requests merged after that time.--open-at <time>: rows open at that time.--created-after <time|ref>: rows created after that time; a thread reference compares numbers, which GitHub assigns in creation order.Each row would also carry
state,html_url,is_draftand, when set, the author, author association, and created/closed/merged times, and the output would reportstate_as_of, the start of the newest complete sync. Existing fields and their order, the default query, and the schema would stay as they are.With such a change, the same question took a median 1.4 s per PR warm (p95 4.3 s over all 3,306 open PRs) on the same copy, and returned the same top 10 as the workaround above in all 40 checks (10 PRs, two questions, warm and cold). I have this ready as a pull request.
What the candidates are worth
Similarity gives candidates, not proof. To see how often the top candidate is the answer, for every open external pull request on the same copy (2,089) I took the most similar pull request by a different author merged after it was created, and checked random samples by comparing both diffs with openclaw/openclaw main at e3415ea8c55 (model-assisted, blind to the score; a second independent blind pass on 10 pairs agreed on 9 of them and on every "already landed" verdict):
One pair in the upper band is borderline between "landed" and "partly landed", hence the range. In two more pairs of the upper band, a different pull request had already landed the change. These rates depend on the embedding model and basis above; with the default
text-embedding-3-smallthey may differ.How far the filters can be trusted
The archive keeps current values, not history. A merge time never changes, so
--merged-afteris exact up tostate_as_ofand can only miss merges after it. Reopening clearsclosed_at, so--open-atcan only include extra rows (a row with an earlier close and reopen), never drop one that was open. These directions, and each row'sstate, assume the closed-thread sweeps have covered the archive since its rows were stored. On the copy above, 0 of 1,100 checked--open-atrows were wrong and all 1,100 checkedmerged_atvalues matched GitHub'smergedAt.