Skip to content

feat(neighbors): filter by state and merge time before ranking - #239

Merged
steipete merged 1 commit into
openclaw:mainfrom
masatohoshino:feat/neighbors-state-time-filters
Oct 17, 2026
Merged

steipete merged 1 commit into
openclaw:mainfrom
masatohoshino:feat/neighbors-state-time-filters

Conversation

@masatohoshino

@masatohoshino masatohoshino commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Closes #238.

What Problem This Solves

Finding similar PRs merged after another PR was opened requires retrieving broad neighbor results and joining a separate thread dump for merge times.

User Impact

User impact: neighbors can filter candidates with --merged-after, --open-at, and --created-after before vector scoring and --limit. Results add state, URL, draft, author, and timestamp metadata. Existing default candidate selection, scores, and fields are preserved; no schema migration is needed.

Why This Change Was Made

Maintainer decision: accept all three opt-in filters and additive metadata. --open-at and --merged-after imply --include-closed; --created-after also accepts a same-repository issue/PR reference.

state_as_of records the start of the newest successful complete open/all sync and closed-thread sweep. It is omitted without a qualifying run; bounded, targeted, failed, and checkpoint runs cannot advance it. It does not certify historical backfill. The clustering guide and CLI help explain legacy sweep gaps, missing historical rows, SQLite millisecond timestamp precision, and the fact that reopening prevents --open-at from reconstructing exact history. Similarity produces candidates to verify before acting.

The contribution is by @masatohoshino. Maintainer completion resolves conflicts against current main, strengthens sync-runner freshness tests, clarifies the freshness contract, and retains contributor credit and the Unreleased changelog acknowledgment. The separate large-ID hydration failure from #238 was already fixed in #255.

Evidence

Focused neighbor, default-output, legacy SQL preservation, time-boundary, freshness, and sync-runner tests pass. Independent Codex autoreview is clean through P3.

Full make check passed on Linux with Go 1.27.2, Node 26.10.0, and GoReleaser 2.18.2: 85.1% coverage, no vulnerabilities, clean formatting/vet/deadcode, CLI and release-script smoke tests, docs/link checks, and six platform snapshot builds. No release was published.

A freshly built CLI also passed against an isolated synthetic SQLite archive. Default results were [201,205]; --open-at 2026-09-10T12:00:00Z returned [201,204]; --created-after '#202' --include-closed returned [204,205,203]; --merged-after 2026-09-10 --limit 1 returned [203] even though other candidates ranked higher before filtering. An exact merge boundary excluded the boundary row. All outputs carried the expected metadata and complete-sync watermark.

Exact-head CI: https://github.com/openclaw/gitcrawl/actions/runs/39008076538 (Linux, macOS, Windows and docs).

@clawsweeper

clawsweeper Bot commented Oct 10, 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. proof: sufficient Contributor real behavior proof is sufficient. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Oct 10, 2026
@clawsweeper

clawsweeper Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

Codex review: blocked before merge.

What this changes

This PR adds three filters before neighbor ranking, thread metadata in results, and a complete-sync freshness timestamp.

Example: Find similar PRs merged after openclaw/openclaw pull request 160462 was created at 2026-09-28T12:59:02Z.

  • Before: Retrieve up to 32,765 neighbors and join a separate thread dump to identify later merges.
  • After: Use --merged-after 2026-09-28T12:59:02Z to return ranked candidates with merge times directly.

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) Strong shipped-CLI evidence and focused behavioral coverage support the patch; overall readiness remains capped by the unresolved product decision.
Proof confidence 🦞 diamond lobster (5/6) Sufficient (terminal): The supplied built-CLI trace exercises neighbors --merged-after on an existing 165,815-thread archive, returning matching ranked candidates with merge metadata and state_as_of; additional reported CLI comparisons cover open-at and unchanged defaults, and the later head changes only tests.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Product

Kind: Feature · Worth it: Needs a maintainer decision · Fix scope: Complete
User problem: Finding similar later-merged PRs requires scoring the entire archive and joining a separate thread dump, potentially losing eligible candidates below the retrieval limit.
Reason: The demonstrated benefit supports the feature, while the new public flags and output semantics require owner direction.

Merge readiness

⛔ Blocked before merge - 1 item remains

Keep this PR open: current main lacks these filters, the contribution demonstrates useful behavior, and no actionable introduced defect was found; the public CLI surface still needs owner direction.

Priority: P2
Reviewed head: 44f46b6549cbd98cbc62986fecf3eb6270c879cc
Owner decision: Required. See Decision needed.

Decision needed

  • Question: Should neighbors expose all three proposed filters and the additive thread metadata and state_as_of output contract?
  • Recommendation: Approve the proposed surface: Accept the opt-in filters and additive output with the documented archive-history limitations.
  • Why: The evidence establishes practical value, but no recorded owner decision authorizes the new public flags and freshness semantics.

Before merge

  • Resolve maintainer decision - Resolve the maintainer decision shown above before merge.

Findings

None.

Agent review details

How this fits together

The neighbors CLI reads cached SQLite thread embeddings, selects eligible vectors, ranks similarity, and returns hydrated thread metadata alongside recorded sync freshness.

flowchart LR
  A[CLI arguments] --> B[Neighbors command]
  C[SQLite archive] --> D[Filtered vectors]
  B --> D
  D --> E[Similarity ranking]
  E --> F[Neighbor output]
  C --> G[Sync freshness]
  G --> F
Loading

Technical review

Best possible solution:

Add approved filters within the existing neighbor query while preserving default selection, scoring, storage, and documented history limitations.

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

Current-main source confirms the missing filters and metadata, and supplied baseline CLI output demonstrates the expensive workaround; this review executed no target code.

Is this the best way to solve the issue?

Filtering in the existing SQL query before decoding and scoring directly addresses the workload without a competing implementation or storage migration.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against bd30790d7735.

Provenance checked

Testing

Proof path: shipped entry point.

Security

None.

Evidence

What I checked:

  • Current main lacks the requested capability: The base command accepts number, limit, threshold, and include-closed; neighbor rows contain identifiers, kind, title, and score without state or merge timestamps. (internal/cli/neighbors.go:19, bd30790d7735)
  • Introduced filters use the existing query owner: Parameterized time and number predicates restrict candidates before vector decoding, scoring, and limiting; unset filters retain the base predicates. (internal/store/vectors.go:199, 44f46b6549cb)
  • Freshness follows complete successful acquisition: StateAsOf excludes checkpoints, failed runs, and targeted scopes; the existing sync producer records closed_sweep_through only after unrestricted successful acquisition. Documentation explains reopening and legacy-sweep limitations. (internal/store/runs.go:207, 44f46b6549cb)
  • Supplied production CLI proof: The full PR body supplies baseline and changed CLI terminal output on an existing 165,815-thread archive, showing matching ranked candidates with merge metadata and state_as_of. The later commit changes only tests, so this production-path evidence remains applicable. Captured body sourceRevision: 0c62a258ef17a3c1be4ec80b03581c2b11e7d0e8fe97a6249fc5b0f1c5e55e7f. (c9310e35663a)
  • Re-review test improvement: The head removes the test that mirrored new SQL fragments and adds returned-row boundary and fractional-time cases; the narrow legacy-query preservation oracle remains. (internal/cli/neighbors_test.go:65, 44f46b6549cb)
  • Feature history and original intent: REST commit patches verify neighbor lookup in 312ee8e and embedding recovery in 0f9386e. refactor: organize CLI commands by responsibility #188 records a behavior-preserving extraction; fix(sync): preserve completed items when another hydration fails #200 distinguishes retry checkpoints from successful freshness. Local content-history search encountered a missing promisor blob; REST reads recovered the relevant introduction evidence. (internal/store/vectors.go:185, 312ee8ed5eac)

Likely related people:

  • Vincent Koc: Raw commit 312ee8e adds internal/store/vectors.go:164 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 312ee8ed5eac; files: internal/store/vectors.go)
  • Peter Steinberger: Raw commit 777a404 adds internal/cli/neighbors.go:233 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 777a404d28a7; files: internal/cli/neighbors.go)

Review metrics

Metric Value Why it matters
Patch distribution Production +166/-17; tests +601; fixtures +342; documentation and changelog +30/-1 Most growth supports returned-row behavior and default-output compatibility; production changes remain in existing CLI and store owners.

Labels

Label changes:

No label changes.

Label justifications:

  • P2: A useful archive-search improvement with demonstrated benefit on large repositories and limited operational blast radius.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • proof: sufficient: Contributor real behavior proof is sufficient.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only; repair and merge need explicit maintainer commands such as @clawsweeper autofix or @clawsweeper automerge.

History

Review history (1 earlier review cycle)
  • reviewed 2026-10-10T16:12:49.464Z sha c9310e3 :: blocked before merge. :: none

Reviewed October 10, 2026, 12:56 PM ET / 16:56 UTC (Revision 2).

steipete added a commit that referenced this pull request Oct 16, 2026
Pass bulk thread IDs through a single JSON parameter so large neighbor, search, and cluster lookups do not exceed SQLite's bind-variable limit. Preserve repository filtering, duplicate and missing-ID behavior, and caller ordering.

Add regression coverage for 32,768 IDs and document large neighbor result limits. Thanks @masatohoshino for reporting the failure in #238; its separate temporal-filter proposal remains open in #239.

Validated with focused regression tests, the full check gates and snapshot artifacts on AWS Crabbox, independent Codex review through P3, and green exact-head CI.
Add opt-in temporal filters before vector decoding and ranking, preserving
existing default selection and scoring. Include additive thread metadata and
a successful complete sync-and-sweep watermark, with explicit historical
coverage, reopening, and timestamp-precision limits.

Verify freshness through real sync-runner paths as well as stored run records.
Closes openclaw#238.

Co-authored-by: Masato Hoshino <g515hoshino@gmail.com>
@steipete
steipete force-pushed the feat/neighbors-state-time-filters branch from 44f46b6 to f5f07aa Compare October 17, 2026 05:56
@steipete
steipete merged commit 00143c5 into openclaw:main Oct 17, 2026
10 checks passed
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. proof: sufficient Contributor real behavior proof is sufficient. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

neighbors cannot tell merged rows from closed ones or filter by time, so a superseded-PR check takes ~18 s per PR

2 participants