Skip to content

fix(graph): keep each observation's best graph score - #1560

Merged
rohitg00 merged 5 commits into
rohitg00:mainfrom
dmazhukov:fix/1558-graph-retrieval-best-score
Oct 16, 2026
Merged

rohitg00 merged 5 commits into
rohitg00:mainfrom
dmazhukov:fix/1558-graph-retrieval-best-score

Conversation

@dmazhukov

@dmazhukov dmazhukov commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Keeps the highest graph score per observation in searchByEntities and expandFromChunks instead of the score of whichever path reached it first. Fixes #1558.

Evidence collection stopped at the first observation it saw, so an observation reachable from several start nodes kept whichever score came first: a weak long path visited early outranked a short strong one, and which score survived depended on the alphabetical order of the entity names. Recall ordering on existing stores changes where that happened.

Rebuilt on #1559. The fix lives in append and adds no index reads, since the evidence it compares is already memoized. Path selection inside a traversal is still Dijkstra's, a re-scored observation moves behind the entries that reached that score first, and a start node's own observations stay at 1.0.

Summary by CodeRabbit

  • Bug Fixes
    • Graph search now keeps the highest-scoring result for each observation when it is reached through multiple entities or paths.
    • Entity search preserves results found directly from a starting entity, while chunk expansion continues to exclude observations used as starting points.
    • Results are ordered by score before the result limit is applied. When scores tie, the first encountered result is retained. Re-scored results appear in their updated position in the ordering.

searchByEntities and expandFromChunks deduplicated observations with a
set shared across every start node, so the first path to reach an
observation fixed its score. A neighbour path (about 0.4) ran before an
entity's own-observation pass and kept its score, so that pass skipped
observations the entity owns outright (1.0). This happens even with a
single matching entity whenever a neighbour shares the observation,
which is the usual result of extraction. Which score survived depended
on traversal order rather than on the graph.

Keep the highest score per observation instead. An upgraded entry takes
the position where its winning score was found, so ties among
observations the old code already scored correctly keep their order.

Signed-off-by: Dmitrii Zhukov <dmitry0983@gmail.com>
@vercel

vercel Bot commented Oct 14, 2026

Copy link
Copy Markdown

@dmazhukov is attempting to deploy a commit to the rohitg00's projects Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 2517edaf-31f4-4f07-a059-beb2e4cebc65
📥 Commits

Reviewing files that changed from the base of the PR and between dc3e4a2 and fd97875.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 17c79595-103d-440a-9214-fb4e6383c8ca
📥 Commits

Reviewing files that changed from the base of the PR and between 739c01c and dc3e4a2.

📒 Files selected for processing: 2
  • src/functions/graph-retrieval.ts
  • test/graph-retrieval.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📝 Walkthrough

Entity search and chunk expansion now retain the highest-scoring candidate for each observation. Both methods sort retained results by score and apply the result limit. Tests cover shared observations, score ties, and ordering after rescoring.

Changes: Graph retrieval scoring

Layer / File(s) Summary
Entity search best-score selection
src/functions/graph-retrieval.ts, test/graph-retrieval.test.ts
searchByEntities replaces a candidate only when it finds a strictly higher score. It adds direct-node evidence before traversal results. Tests cover shared observations, stronger routes, and start-node scores.
Chunk expansion best-score selection
src/functions/graph-retrieval.ts, test/graph-retrieval.test.ts
expandFromChunks replaces a candidate only when it finds a strictly higher score, while excluding requested observations. Tests cover duplicate observations, nearer-path selection, ties, and result ordering.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~15 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: rohitg00

Merge Risk: ⚪ Minimal · up to dc3e4

This change makes graph retrieval keep each observation's best score, so recall ordering may shift on existing stores, as the description states. No merge-blocking risk was identified.

Pre-merge checks: Passed 5
Check name Status Explanation
Linked Issues check Passed Issue #1558 requires best-score retention in searchByEntities and expandFromChunks. In src/functions/graph-retrieval.ts, append stores results by observation ID, ignores lower or equal scores,…
Out of Scope Changes check Passed The changes modify graph retrieval deduplication and scoring, plus focused graph retrieval tests. These changes directly implement issue #1558. No unrelated implementation or test changes are shown.
Docstring Coverage Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: retaining the best graph score for each observation.
Full details: Linked Issues check

Explanation

Issue #1558 requires best-score retention in searchByEntities and expandFromChunks. In src/functions/graph-retrieval.ts, append stores results by observation ID, ignores lower or equal scores, and replaces lower scores with higher scores. The replacement preserves the winning path data and moves the entry to its latest map position. Direct entity observations are added at score 1 and excluded from later traversal. test/graph-retrieval.test.ts covers query-order independence, closest-path selection, rescoring order, equal-score stability, and score 1 for start-node observations.

Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2 files.

✨ Finishing Touches: 💡 1
🛠️ Fix failing CI checks: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

The existing re-scoring test only covers a start node upgrading a path hit. Add a path upgraded by a better path in searchByEntities and the same case in expandFromChunks, asserting the upgraded score so that a missing upgrade fails as well as a missing reorder.

Signed-off-by: Dmitrii Zhukov <dmitry0983@gmail.com>
@dmazhukov

Copy link
Copy Markdown
Contributor Author

Reconciled with #1559 on reconcile/1559-1560: a merge of this branch and 0225c65 whose diff against #1559 is only this PR's best-score map, applied inside #1559's budgeted loops (the counter, yields and abort checks are unchanged). Graph and hybrid-search tests and the full suite pass on it; the only failures are the existing openclaw-connect ones.

With #1559's early own-observation pass, the start-node fixture here no longer tests anything, so 739c01c adds a path-over-path upgrade and the same case for expandFromChunks; that commit does not depend on #1559. Once #1559 lands I will merge main here and resolve to that branch's content.

Base rewrote graph-retrieval.ts and its tests for the bounded index reader
(rohitg00#1559), so both conflicted files take the base version here; the best-score
fix is re-applied on top of the new retrieval path in the next commit.

Signed-off-by: Dmitrii Zhukov <dmitry0983@gmail.com>
Evidence collection stopped at the first observation it saw, so an
observation reachable from several start nodes or several destination
nodes kept whichever score was produced first. A weak long path visited
early could outrank a short strong one, and which score survived
depended on start-node ordering.

Scores now key on the observation id and are replaced only by a strictly
higher one; a re-scored observation moves behind the entries that reached
that score first, so ties stay deterministic. A start node's own
observations stay at 1.0, as they were when the first-seen set pinned
them.

Signed-off-by: Dmitrii Zhukov <dmitry0983@gmail.com>
@dmazhukov

Copy link
Copy Markdown
Contributor Author

Rebuilt on top of #1559, which rewrote graph-retrieval.ts around the bounded index reader, so the earlier diff no longer applied. Base is merged in, not rebased; the fix is now one guard in GraphQuery.append (src/functions/graph-retrieval.ts:165).

#1559 closed the start-node half of #1558: own observations are appended first at 1.0. The rest is still open on main - the new tests fail on 7bf9c53 whenever the weaker start node sorts first (Alpha before Zeta, a_far before b_near).

Index reads are unchanged: source() memoizes into this.evidence and the score comparison short-circuits before it. Full suite green, tsc error count unchanged at 26.

…ieval-best-score

Signed-off-by: Dmitrii Zhukov <dmitry0983@gmail.com>
@rohitg00
rohitg00 merged commit 6a182c4 into rohitg00:main Oct 16, 2026
11 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[graph] searchByEntities scores depend on start-node order: visitedObs is first-come-first-served and never upgraded

2 participants