Skip to content

improvement(knowledge): rank inside the permitted set for organization search - #7996

Merged
waleedlatif1 merged 2 commits into
stagingfrom
improvement/search-permitted-set-planner
Sep 19, 2026
Merged

waleedlatif1 merged 2 commits into
stagingfrom
improvement/search-permitted-set-planner

Conversation

@waleedlatif1

@waleedlatif1 waleedlatif1 commented Sep 19, 2026

Copy link
Copy Markdown
Collaborator

Summary

Organization search indexes grant most documents to one mailbox, channel, or file owner, so a typical member can read about 0.1% of the index. Both retrieval legs searched the whole index and checked access afterwards. The vector walk then post-filtered nearly every neighbour away, and common keyword terms ranked matches the member could never see.

  • Resolve the permitted set once. retrieveKnowledgeSearch calls resolvePermittedDocuments before both legs for live user scopes (a user scope with an access provider and no explicit documentIds). It returns bounded (documents plus their connector) or unbounded. The set is resolved with the same candidate predicate both legs apply, so restricting a leg to it can only narrow.
  • Vector. bounded ranks the permitted chunks exactly and skips the HNSW walk. unbounded runs the walk only, with no second tractability probe.
  • Keyword. bounded reads the permitted chunks directly through embedding_keyword_search_document_idx, using an OFFSET 0 fenced subquery aliased as the table. Base, enabled, tsquery and tag checks are applied outside it. The existing visibility CTE still re-applies the candidate predicate.
  • Probe SQL (user scopes).
    • It counts the documents the caller's tokens reach inside the requested bases, up to the limit. It reads doc_acl_gin_idx (deleted_at IS NULL AND acl && tokens) inside an OFFSET 0 fence and applies the base check to the rows the index returns, so the planner cannot swap to a whole-base heap scan.
    • The count is base-scoped because the baseline tokens (pub/ws/org) are shared by every tenant; a global count would saturate for nearly every caller.
    • Only when the count is under the limit does it materialize those rows and apply the base, state and full candidate access conditions in memory.
    • Over the limit, a saturated sentinel row reports unbounded.
    • Resolved scopes keep the plain form.
  • Members-mode access clause. The row-value IN (subquery) is now a correlated EXISTS, served by the observation PK (document_id, member_id). Semantics are identical: every compared column is NOT NULL, and the clause sits inside the connector EXISTS, so document.connector_id is non-null there. PostgreSQL no longer hashes every observation once per statement.
  • Excluded sources and timeouts.
    • Sources excluded by live authorization are dropped from the permitted set on refill; uploads with a null connector stay.
    • An empty set short-circuits both legs.
    • If the vector budget runs out during resolution, the result is unbounded and the vector leg is marked timed out; the keyword leg is not failed.
  • Diagnostics. New stage permitted_documents and new metadata permittedDocuments / permittedDocumentCount.

Evidence

Server-side execution time on a restored copy of a large organization search index, measured with the same members and queries on main and on this branch:

  • Typical member, keyword:
    • common term: 1640 ms → 42 ms;
    • rare term: 8 → 23 ms.
  • Typical member, vector:
    • Before: the HNSW walk found 0–16 of 1600 candidates after 0.7–3.4 s, then the tractability probe timed out at 600 ms.
    • Now: permitted set 37 ms + exact ranking 17 ms + rerank 53 ms, with complete results.
    • Replay recall went from 13–14 of 20 to 20 of 20.
  • Member whose tokens reach several times the probe limit:
    • hydration and rerank each 142–188 ms → ~1 ms, from the members-mode EXISTS;
    • probe 2.0 s warm / 11.8 s cold → ~200 ms;
    • rare keyword 319 → 2 ms.
  • Unchanged: the unbounded common-term keyword path stays at ~2.3 s. A follow-up addresses it.
  • Calibration:
    • Exact vector ranking costs ~3 µs per permitted chunk: 7 ms at 1k documents, 513 ms at 100k.
    • The HNSW walk only fills when a member can read roughly 20% or more of the index, so the bounded/unbounded split stays at the existing VECTOR_PROBE_DOCUMENT_LIMIT (100k documents).
    • Keyword: the direct read and the text-index intersection cost about the same on common terms, but the planner misplanned the index form for real members (147–180 ms vs 23–42 ms). Bounded keyword therefore always uses the direct read.

Type of Change

  • Improvement (performance)

Testing

  • 11 new planner unit tests, plus a real-Postgres test: VECTOR_PROBE_DOCUMENT_LIMIT + 1 reachable documents in a sibling base must leave the target base bounded. The unit tests cover bounded exact ranking skips the walk; empty sets; no re-probe when unbounded; keyword confinement; excluded-source refill; saturation; budget exhaustion; resolved once before both legs. Each was confirmed to fail against a broken planner.
  • 2965 knowledge unit tests pass; type-check, lint, check:api-validation and check:audits are clean.
  • Real-Postgres knowledge ACL suite: 501/502. The one failure is a pre-existing organization-personal-tokens test that also fails on staging.

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)

@vercel

vercel Bot commented Sep 19, 2026

Copy link
Copy Markdown

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

1 Skipped Deployment
Project Deployment Actions Updated
Image docs Skipped Skipped Sep 19, 2026 12:50am UTC

Request Review

@greptile-apps

greptile-apps Bot commented Sep 19, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

The PR appears safe to merge; no outstanding previous finding or new actionable issue remains.

Summary

This PR improves organization-scoped knowledge retrieval by resolving a caller’s permitted document set before ranking.

  • Uses exact vector ranking for bounded permitted sets and retains HNSW traversal for unbounded sets.
  • Restricts bounded keyword matching to permitted documents before ranking.
  • Reworks the user-scope probe to count reachable documents within requested knowledge bases.
  • Optimizes members-mode observation checks with a correlated lookup.
  • Adds retrieval diagnostics and planner-focused unit and PostgreSQL integration coverage.
Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Knowledge search request] --> B{Live user scope without explicit document IDs?}
  B -->|No| C[Use existing resolved-scope retrieval]
  B -->|Yes| D[Resolve permitted documents in requested bases]
  D --> E{Permitted set bounded?}
  E -->|Yes| F[Exact vector ranking within permitted documents]
  E -->|Yes| G[Keyword matching within permitted documents]
  E -->|No| H[HNSW vector traversal with candidate access checks]
  E -->|No| I[Text-index keyword search with candidate access checks]
  F --> J[Live source authorization]
  G --> J
  H --> J
  I --> J
  J --> K[Hydrate authorized results]
Loading

Reviews (3) · Last reviewed commit: "fix(knowledge): count a caller's reach i..."

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 5 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
@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 19, 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.

All reported issues were addressed across 5 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
@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 19, 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 5 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 305da02 into staging Sep 19, 2026
35 checks passed
@waleedlatif1
waleedlatif1 deleted the improvement/search-permitted-set-planner branch September 19, 2026 01:06

This branch was previously deployed

1 inactive deployment
Preview a63f19c3 Deployed Sep 19, 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