Skip to content

fix(knowledge): give the reach count the probe's share of the deadline, not the leg's - #8111

Merged
waleedlatif1 merged 2 commits into
stagingfrom
fix/reach-count-budget
Sep 21, 2026
Merged

waleedlatif1 merged 2 commits into
stagingfrom
fix/reach-count-budget

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

  • Counting a caller's reach (how much of the bases they can read, which picks between one graph walk and a per-source search) reads as many index entries as they reach. On a large index a broad member's count consumed the whole vector budget, the leg had no time left to walk, and the search reported the vector leg timed out; the count's stage was also measured twice (an outer wrapper around an inner measured statement), so diagnostics showed it at double its real cost
  • The count now runs under the probe's share of the deadline (the same cap the permitted-documents probe already uses), never the whole leg's. A count that runs out of that share answers nothing for this search: the leg keeps its remaining time to walk, is not marked timed out, and nothing is remembered, so the next search counts again
  • The reach memo now holds for an hour instead of five minutes. Reach moves slowly and only chooses a ranking strategy; a stale answer costs speed, never access. On a multi-instance deployment this turns a per-instance, per-five-minutes cold count into a rare one
  • The duplicated stage wrapper is removed, so permitted_documents reports the statements' real time

Type of Change

  • Bug fix

Testing

  • The existing timed-out-count test now also asserts the leg's own deadline is not marked spent; it fails when the count's timeout is judged by the leg's budget instead of its own share
  • The small-index count test asserts the count's statement timeout is at most the probe budget; it fails with the cap removed
  • vitest over lib/knowledge and app/api/knowledge (191 files, 3304 tests), bun run lint, bun run check:audits and native tsc --noEmit pass

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

…e, not the leg's

Counting a caller's reach reads as many index entries as they reach, so on a large index it took the whole vector budget and left the leg no time to walk; the search then reported the leg timed out with the count's stage measured twice. The count now runs under the probe's share of the deadline, a count that runs out answers nothing for this search without marking the leg, and the reach memo holds for an hour: reach moves slowly and a stale answer costs speed, never access.
@vercel

vercel Bot commented Sep 21, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated
docs Skipped Skipped Sep 21, 2026 6:27pm UTC

Request Review

@greptile-apps

greptile-apps Bot commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

The PR appears safe to merge; the previous stale-strategy concern was resolved by restoring the five-minute cache TTL, and no new actionable defects remain.

Summary

This PR bounds reach counting to the probe’s share of the search deadline and preserves the remaining vector-leg budget when only that count times out. Follow-up changes restore the reach-cache TTL to five minutes, distinguish an expired leg from an expired count share, and add deadline-accounting tests.

  • Caps reach-count database work using a child search budget.
  • Avoids memoizing an inconclusive timed-out count.
  • Preserves partial-retrieval reporting when the parent vector deadline expires.
  • Removes duplicate permitted_documents stage measurement.
  • Keeps the reach-strategy cache at five minutes to limit stale planning decisions.
Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Resolve caller reach] --> B[Create probe-capped count budget]
  B --> C{Reach count finishes?}
  C -->|Yes| D[Choose broad or narrow strategy]
  D --> E[Memoize strategy for five minutes]
  C -->|Count share expires| F[Use broad fallback for this search]
  F --> G[Do not memoize]
  C -->|Parent leg expires| H[Mark vector leg timed out]
  G --> I[Continue retrieval with remaining leg budget]
  H --> J[Return partial retrieval status]
Loading

Reviews (2) · Last reviewed commit: "fix(knowledge): keep the reach memo at f..."

Comment thread apps/sim/lib/knowledge/search/queries.ts Outdated

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 2 files

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

Comment thread apps/sim/lib/knowledge/search/queries.ts Outdated
… leg during the count as short

A stale strategy costs recall at the margin, so the memo stays at five minutes now that a count costs at most the probe's share. A leg whose own deadline passes during the count reports short, as it did before, instead of failing the search.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 2 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Re-trigger cubic

@waleedlatif1
waleedlatif1 merged commit d03d33e into staging Sep 21, 2026
34 checks passed
@waleedlatif1
waleedlatif1 deleted the fix/reach-count-budget branch September 21, 2026 19:15
@waleedlatif1 waleedlatif1 mentioned this pull request Sep 21, 2026

This branch was previously deployed

1 inactive deployment
Preview 1ea4c24d Deployed Sep 21, 2026 by vercel[bot]
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