Skip to content

fix(retrieval): keep scope_id under node budget; honest scoped truncation; validate depth - #145

Merged
mrchatam merged 1 commit into
mainfrom
fix/p44-project-graph-scope-id
Sep 26, 2026
Merged

mrchatam merged 1 commit into
mainfrom
fix/p44-project-graph-scope-id

Conversation

@mrchatam

Copy link
Copy Markdown
Owner

Problem (verified on main 908d498)

  1. scope_id disappears whenever the node budget fills. collectProjectNodes returns early as soon as max_nodes is reached. The memberScopeIndex fill (and the sort) only ran when every entity fit. Probe with 1 scope + 5 member tasks: MaxNodes 3/5/6 → 0 nodes carry scope_id; 100 → 5. An exact fit (6 nodes, truncated=false) loses them as well. In practice the GUI's scope clustering never kicks in for projects above the 500 default. memberScopeIndex also scanned every scope_member link, with no bound.
  2. Scoped mode misreports truncation. Scope + 4 members with MaxNodes=5 returns truncated=true even though nothing was left out. With MaxNodes=3 it returns truncated=true but total_entities == len(nodes), so the GUI shows 0 omitted.
  3. depth in mode=project&scope=… was silently ignored when invalid and silently capped above 2. OpenAPI still said "neighborhood mode only".

Fix

  • collectEdgesForNodes fills ScopeID from the first outgoing scope_member → scope link it already reads per included node. This happens before the included-target filter, so it still works when the scope node itself falls outside the budget. The project path no longer does a global scan. memberScopeIndex is kept only for Neighborhood, which is unchanged.
  • collectProjectNodes wraps an unsorted collector and sorts once on every path, through a shared projectGraphSortLess.
  • Scoped mode:
    • Budget is checked before lookupEntity, so no extra DB read happens for members that get dropped.
    • truncated only reflects real drops.
    • total_entities = len(nodes) + distinct dropped (a lower bound, commented as such).
  • HTTP: when scope is set, depth must be an integer 1..2 or the request gets 400 VALIDATION_ERROR. Without scope, depth is still ignored, as before, so existing clients keep working. The OpenAPI description is updated.

Tests

  • internal/retrieval/scope_graph_budget_test.go:
    • MaxNodes 3/5/6/100: every task node has the scope id. At 3/5 the scope node is excluded, which proves the case that was broken.
    • Scoped exact fit → not truncated, total=5; budget 3 → truncated with total > len(nodes).
    • Engine-level depth defaults and cap.
  • internal/httpapi/httpapi_test.go TestGraphProjectModeDepthValidation: with scope, depth=abc/3 → 400 and depth=1/2/none → 200. Neighborhood depth=8 still returns 200.

The scope_id, truncation and HTTP tests fail on main and pass here. CGO_ENABLED=1 go test ./... passes locally.

Reviewer note (Low, not changed here): in scoped mode, a member dropped in the seeding loop doesn't set truncated directly. It gets set right after, because the depth ≥ 1 walk from the scope node runs into the same dropped members, and the tests cover this. A follow-up could set truncated = len(dropped) > 0 explicitly.

Patch produced by the Kilo worker (Nemotron 3 Ultra; task kw-20260926-192624-a6d6) and independently reviewed and tested by the supervisor. It took 2 revisions: the first fixed compile errors, a real bug (ScopeID was assigned after the included filter, which would have kept the bug alive in truncated graphs), the dropped-member lookups, and the depth validation scope. The second removed a leftover duplicate function.

@coderabbitai review

…tion

- scope_id is derived per included node from the outgoing links already read
  in collectEdgesForNodes (before the included-target filter), so truncated and
  exact-fit project graphs keep scope clustering; no unbounded scope_member scan.
- nodes are sorted once on every return path.
- scoped mode: truncated only when a candidate is actually dropped;
  total_entities = included + distinct dropped (lower bound); no lookup for
  members beyond the budget.
- GET /v1/graph mode=project&scope=…: depth must be 1..2 (400 otherwise);
  OpenAPI depth description updated.

Patch produced by the Kilo worker (Nemotron 3 Ultra), independently reviewed.
@coderabbitai

coderabbitai Bot commented Sep 26, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 40 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository: mrchatam/Trace/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 95a003a4-f8a9-4224-96a1-331adfaf2b6f

📥 Commits

Reviewing files that changed from the base of the PR and between 908d498 and 687e8d3.

📒 Files selected for processing (5)
  • api/openapi.yaml
  • internal/httpapi/handlers_retrieval.go
  • internal/httpapi/httpapi_test.go
  • internal/retrieval/project_graph.go
  • internal/retrieval/scope_graph_budget_test.go

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.

@mrchatam
mrchatam merged commit ca06522 into main Sep 26, 2026
6 checks passed
@mrchatam
mrchatam deleted the fix/p44-project-graph-scope-id branch September 30, 2026 21:15
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.

1 participant