Skip to content

improvement(knowledge): resolve connector and member state once per search - #8040

Merged
waleedlatif1 merged 30 commits into
stagingfrom
improvement/search-acl-expiry
Sep 20, 2026
Merged

waleedlatif1 merged 30 commits into
stagingfrom
improvement/search-acl-expiry

Conversation

@waleedlatif1

@waleedlatif1 waleedlatif1 commented Sep 19, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Carry each chunk's source and ACL onto the ranking projections (embedding_search, embedding_keyword_tin), set by each projection's own trigger under a share lock on the document and fanned out when a document's source or ACL changes; script migration 0021 installs the triggers, backfills both projections in keyset pages, and builds a partial GIN on each acl and a partial btree on the vector projection's source concurrently
  • Decide candidate readability on the row that is scored: the whole-graph walk, the per-source walks, the sliced sources' exact ranking and the Tin keyword page all test the mirrored source and ACL instead of joining document per visited or ranked chunk. Content is still read under the full predicate at hydration, and result metadata under the scope the results were read under
  • Let a resolved scope's reach choose its plan without enumerating documents: a broad reader walks the whole graph once (and again with a wider scan if its neighbourhood is mostly unreadable); a narrow reader walks every source it reads whole in that source's own index — earned at a thousand documents now that a walk costs milliseconds — and ranks the residual exactly; a narrow reader's keyword page is one wide Tin window ranked once
  • Resolve each connector's state once per search — deletion, archival, pending access rewrite, integration approval and access mode are facts about a connector, not a document — and the caller's member identities with it, so candidate ranking checks ids instead of proving connector state per row
  • Choose the vector plan by the caller's reach: a caller who reaches at least a quarter of the bases' documents walks the whole graph once (pgvector post-filters, so at that share almost every neighbour is theirs); a narrower unbounded caller has each readable source searched on its own; a bounded caller who is a member of an indexed source walks that source and ranks the rest exactly. The share is counted once when the probe saturates and remembered with the saturation; a broad walk that comes back short falls back to per-source without discarding what it found
  • Give a source its own partial vector index once it holds enough documents to need one; carry the chunk's source onto the vector projection (embedding_search.connector_id), maintained by triggers, with script migration 0021 backfilling in independently committed keyset pages under document share locks
  • Ask a live-proof source (installation-backed GitHub, admin-mode Confluence) for the caller's grants only when a page of ranked candidates contains one of its documents, once per search, in the provider's bounded pages; a source the caller turns out not to hold is excluded and the pages rebuilt. Content is read under the full predicate, and result metadata under the scope the results were read under

Behaviour

The on-row predicate admits a superset of the per-row document predicate — a member's source whole, and mirrored ACLs within eligible sources — and never refuses what that predicate admits (asserted against a database); whatever it admits beyond that is refused at hydration. The resolved connector predicate admits exactly the documents the per-row predicate admits. Hydration re-reads each connector's lifecycle and approval per row, so a source deleted, archived or unapproved while a search runs stops answering at the gate that returns content. A live-proof source is still proven before its content is read — once per search rather than once per page. Uploads are ranked even when every connector source is walked. v1 fails a partial retrieval as before.

Type of Change

  • Improvement

Measurements

On a production-shaped copy, server time p50 by query class (common / medium / rare) and recall@20 against exact ranking, for a reader who reaches 63% of the index and one who reaches 3%:

reader before after recall before → after
broad (63%) 149 / 96 / 92 ms 99 / 52 / 50 ms 0.83 → 0.95
narrow (3%) 1049 / 751 / 575 ms 213 / 171 / 172 ms 0.98 → 0.98

The narrow reader's after column sums the vector leg's statements (four source walks, the sliced sources' exact ranking, the rerank); those run concurrently on separate connections, so the leg's own critical path is about 95 ms, and its keyword leg is 32–48 ms on terms the reader can read anywhere (213 ms only where they can read a term nowhere and both windows run). A broad reader whose query lands in a neighbourhood they mostly cannot read completes at 218–244 ms warm; cold, that walk is bound by a heap fetch per visited tuple, which only an index that carries the ACL would remove.

The decisive experiment behind the design: the same graph walk, same scan budget, same qualifying neighbours cost 6,726 ms with the permission test joined to document per visited tuple and 64 ms with it on the row.

Testing

Tested manually. Unit suites for knowledge and the v1/internal knowledge routes pass (3,265 tests); a PostgreSQL test asserts the per-query and per-row predicates admit the same documents over admin, members, workspace and upload rows; the real-database knowledge integration suites (ACL end-to-end, KB-block fan-out, connector permissions) pass. New tests cover the reach-share decision and its caching, the broad-walk fallback and its budget, the lazy grant resolution (never asked without a gated candidate; asked once with one), exclusion and refill after a denied source, the upload slice, the saturated-slice walk, source index builds and DDL identifier guards, and the v1 partial-retrieval failure — each verified to fail without its change. The 0021 triggers and backfill were exercised against a database: insert fills the source, a document changing hands fans out to enabled chunks, a re-enabled chunk re-reads its source, the share-locked backfill fills a cleared row and reruns as a no-op. bun run lint, bun run type-check, bun run check:audits and bun run check:migrations origin/staging 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)

@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
docs Skipped Skipped Sep 20, 2026 12:52pm UTC

Request Review

@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 21 files

Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.

Fix all with cubic | Re-trigger cubic

Comment thread packages/db/schema.ts Outdated
Comment thread packages/db/script-migrations/0020_document_acl_valid_until.ts Outdated
@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 new actionable issue was found, and all previous findings are resolved.

Summary

This PR restructures knowledge retrieval so connector eligibility and member state are resolved once per search, while source and ACL data are mirrored onto ranking projections for row-local authorization filtering.

  • Adds access-plan resolution and projection-level candidate predicates while retaining full authorization at hydration.
  • Selects vector-search strategies according to caller reach and source-index availability, with timeout-aware recovery paths.
  • Adds per-source vector-index lifecycle management.
  • Adds replay-safe migrations, triggers, and bounded backfills for projection source and ACL fields.
  • Preserves explicit partial-retrieval handling, including failure for the v1 API surface.
  • The change since the previous review only consolidates vector scan settings with each query's deadline-setting statement and updates the corresponding integration expectation.
Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Resolve caller access scope] --> B[Resolve connector eligibility and member identities]
  B --> C{Choose retrieval plan}
  C -->|Broad reach| D[Whole-graph vector walk]
  C -->|Readable indexed sources| E[Per-source vector walks]
  C -->|Residual sources and uploads| F[Exact vector ranking]
  C --> G[Tin keyword ranking]
  D --> H[Merge and rerank candidates]
  E --> H
  F --> H
  G --> H
  H --> I[Resolve required live-source grants]
  I -->|Denied source| C
  I -->|Allowed| J[Hydrate under full access predicate]
  J --> K[Read metadata under retrieval access scope]
  K --> L[Return complete or explicit partial status]
Loading

Reviews (20) · Last reviewed commit: "test(knowledge): expect the scan setting..."

Comment thread apps/sim/lib/knowledge/connectors/member-observations.ts Outdated
Comment thread apps/sim/lib/core/config/env.ts Outdated
…in its own index

Two costs dominated organization search, and both came from deriving per candidate
what is true of a whole source.

Connector state — deletion, archival, a pending access rewrite, the organization's
integration approval, the access mode — is a fact about a connector, so a search
resolves it once and filters candidates by the resulting ids. For a member reading
~25k documents, the candidate predicate's connector lookup drops from one per
document examined to one per document of a source that still needs this request's
live proof: 24,571 evaluations to 610, and the access check from ~390ms to ~160ms.

Ranking then follows sources. pgvector post-filters, so one walk over every source
spends its scan budget on the sources a caller cannot read: measured recall for a
member reading half an index ranged from 0.00 to 1.00, averaging 0.80 over nine
queries, two of which returned none of the exact page. A member of a source reads
essentially all of it, so that source is walked through an index covering it alone,
built after a sync grows it past the threshold and dropped with the connector.
Every other source is sliced — mirrored permissions give a caller their own mail,
their own files — and those slices are ranked exactly in one statement. Recall over
the same queries rises to 0.95 with none below 0.75.

Retrieval never waits on an index existing: a source without one is ranked exactly,
so a build that is skipped, fails, or has not happened yet costs recall nothing.
…he rest of the plan

A members-mode document is readable while one of the caller's active members observes
it, freshly, and which members those are is a fact about the caller. Resolving them
with the connectors — one query, one plan — turns each candidate's check into a lookup
on the observation key instead of a join to the member behind it, and lets the vector
planner read the sources the caller belongs to from the same resolution rather than
asking again.
- ask a live-proof source for the caller's grants once, before either leg, and only when the scope actually reads one, instead of per candidate page
- drop the per-page source exclusion and refill loop that followed from the per-page proof
- read search result metadata under the stored predicate the rows already passed
- end candidate paging on a short page whether or not the leg reorders its results
- batch the embedding_search.connector_id backfill in independently committed keyset pages, matching the other projection backfills
@waleedlatif1
waleedlatif1 force-pushed the improvement/search-acl-expiry branch from 575cb52 to edf9679 Compare September 20, 2026 06:03
@waleedlatif1 waleedlatif1 changed the title improvement(knowledge): decide mirrored-permission freshness from the document row improvement(knowledge): resolve connector and member state once per search Sep 20, 2026
@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 20, 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.

Comment thread apps/sim/lib/knowledge/search/queries.ts Outdated
Comment thread apps/sim/lib/knowledge/access/predicate.ts
… hydration

- fall back to a graph walk when the sliced sources hold more readable documents than one exact ranking may enumerate, since that enumeration has no order and would otherwise rank an arbitrary subset
- read content under the full predicate, which re-reads each connector's own lifecycle and approval, so a source deleted, archived or unapproved mid-search stops answering at the gate that returns content
@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 20, 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.

… its candidates is read

Resolving every gated source's grants up front charged a search for sources it never ranked. The grants are now resolved on first need — when a page of ranked candidates actually contains one of those sources — and memoized for the rest of the search, so a scope that ranks none never asks and one that ranks many asks once.
@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 20, 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 27 files

You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.
Tip: instead of fixing issues one by one fix them all with cubic

Re-trigger cubic

Comment thread packages/db/script-migrations/0021_embedding_search_connector.ts
Comment thread packages/db/script-migrations/0021_embedding_search_connector.ts Outdated
Comment thread apps/sim/lib/knowledge/search/source-vector-indexes.ts Outdated
Comment thread apps/sim/lib/knowledge/access/predicate.ts
Comment thread apps/sim/lib/knowledge/search/queries.ts Outdated
…ource fan-out, serialize a source's index build

- an observation vouches for a document only from a member of the document's own connector: a document that changed hands keeps its old observations, which no longer carry it as a candidate
- a source whose search runs out of budget marks the leg partial while the other sources' results stand
- a source's index is built under a session lock on the one reserved connection that also builds it, so two syncs cannot interleave the invalidity check and the drop; 0021 drops an invalid leftover before building, and analyzes both projections after the backfill
- a broad reader's walk widens when it found fewer candidates than the smallest pool worth reranking
@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 20, 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.

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

The rerank and hydration of whatever is found — the first walk's candidates at least — keep the rest, so a wider walk that runs out of its share can no longer take the leg's results with it.
@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 20, 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 27 files

You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.

Fix all with cubic | Re-trigger cubic

Comment thread apps/sim/lib/knowledge/access/predicate.ts Outdated
Comment thread packages/db/script-migrations/0021_embedding_search_connector.ts
Comment thread apps/sim/lib/knowledge/search/queries.ts Outdated
Comment thread apps/sim/lib/knowledge/search/queries.ts Outdated
…rrow reader's keyword window stepwise, apply scan settings with the deadline

- an on-row walk keeps walking, up to a 100k-tuple cap, until its limit is met; the separate wider walk and its diagnostic are gone
- the on-row predicate tests the mirrored ACL alone — a member's source is no longer admitted whole, so a document re-owned after its chunk was mirrored is refused at the row
- a reach count that ran out of time decides that search only; it is not remembered
- a narrow reader ranks the narrowest keyword window first and widens to the wide one only when the page is short; resolved scopes leave the widest window short instead of ranking every match
- the HNSW scan settings ride in the deadline statement, one round trip fewer per vector statement
@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 20, 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 28 files

Confidence score: 5/5

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

You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.

Re-trigger cubic

@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 20, 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 29 files

Confidence score: 5/5

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

You've manually re-run cubic several times on this PR. Each manual re-review checks the full PR again and counts toward your usage quota. To preserve your usage limits, we recommend letting cubic automatically review new commits.
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.

Re-trigger cubic

@waleedlatif1
waleedlatif1 merged commit 9573ecc into staging Sep 20, 2026
34 checks passed
@waleedlatif1
waleedlatif1 deleted the improvement/search-acl-expiry branch September 20, 2026 16:24

This branch was previously deployed

1 inactive deployment
Preview 265ed271 Deployed Sep 20, 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