Skip to content

fix(knowledge): read an unfilled keyword candidate's source from its document - #8176

Merged
waleedlatif1 merged 3 commits into
stagingfrom
fix/search-unfilled-row-source-from-document
Sep 22, 2026
Merged

waleedlatif1 merged 3 commits into
stagingfrom
fix/search-unfilled-row-source-from-document

Conversation

@waleedlatif1

@waleedlatif1 waleedlatif1 commented Sep 22, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • The Tin keyword page took each candidate's connectorId from its projection row. Rows the source/ACL fill hasn't reached carry connector_id = NULL, so a chunk from a live-proof source (GitHub installation, Confluence site) wasn't recognized as gated: the grants weren't resolved, hydration ran without them, and the document was hidden from a member who can read it (fails closed, no leak). ACL writes used to repair those rows incidentally; now that unchanged ACLs aren't rewritten and unfilled rows are left to the backfill, they stay NULL until the fill reaches them
  • Until embedding_keyword_tin is filled, the page's unfilled rows (acl IS NULL) take their source from the document: a primary-key read per row of the page, after its LIMIT, never per ranked row. Filled rows keep the row's source
  • The keyword excluded-sources filter now asks the document while the projection is unfilled, mirroring the vector leg, so a denied source's unfilled rows stop taking slots on a rebuilt page
  • The filled path is untouched: the statement is byte-identical before/after and so is its plan
  • The vector leg already re-reads a page's identities from documents while embedding_search is unfilled (vector.page), so it needed no change; the new test covers it
  • Fixes staging's PostgreSQL integration jobs: after fix(knowledge): stop document ACL changes from rewriting unfilled projection chunks #8175 the trigger no longer writes unfilled chunks, which broke two fix(knowledge): refresh unchanged mirrored ACLs without firing the projection fan-out #8169 assertions in sync-persistence.postgres.test.ts. Those tests now count the document writes that fire the fan-out trigger (0 for an unchanged ACL) and expect a changed ACL on filled chunks only; reverting fix(knowledge): refresh unchanged mirrored ACLs without firing the projection fan-out #8169's fix still turns 2 red
  • Not changed: per-source HNSW walks key on embedding_search.connector_id, so an unfilled row with a NULL source is only reachable through the sliced/uploads walk until the fill reaches it (recall, not access)

Type of Change

  • Bug fix

Testing

  • New unfilled-projection-source.integration.ts (real migrated Postgres, real GitHub installation grant fixture, Tin operators shimmed since the test DB has no Tin): an unfilled keyword row and an unfilled vector row with NULL source reach the member holding the grant and stay hidden from one without it; the filled path behaves the same; after a denial the rebuilt keyword page no longer carries the denied source's unfilled row
  • Mutation-checked: reverting queries.ts → 2 red; row-sourced page only → 2 red; row-only exclusion → 1 red; making the vector page trust the row source → 1 red
  • EXPLAIN (filled, before vs after): identical statement and plan. Unfilled: the document lookup runs once per page row (loops = page rows)
  • bun run lint, bun run check:audits, type-check, lib/knowledge/search unit tests 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 22, 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 22, 2026 11:33pm 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.

No issues found across 3 files

Confidence score: 5/5

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

Re-trigger cubic

@greptile-apps

greptile-apps Bot commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

The PR appears safe to merge; the updated query preserves filled-row behavior and correctly resolves document sources for unfilled candidates.

Summary

This PR fixes keyword-search authorization for projection rows whose source and ACL backfill has not completed.

  • Resolves an unfilled keyword candidate’s source from its document before live-source authorization.
  • Applies excluded-source filtering against the document only for unfilled rows while preserving the filled-row query path.
  • Adds PostgreSQL integration coverage for authorized, unauthorized, filled, unfilled, and denied-source cases.
  • Updates persistence assertions to reflect that changed ACLs propagate only to already-filled projection rows.
Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Rank keyword candidates] --> B{Projection fill complete?}
  B -->|Yes| C[Use projection connector ID]
  B -->|No| D{Candidate ACL is null?}
  D -->|No| C
  D -->|Yes| E[Read connector ID from document]
  C --> F[Apply excluded-source filter]
  E --> F
  F --> G[Resolve live-source grants]
  G --> H[Hydrate authorized results]
Loading

Reviews (2) · Last reviewed commit: "fix(knowledge): read the document only f..."

…document

The Tin keyword page took each candidate's source from its projection row. A
row the source and ACL fill has not reached carries a NULL source, so a chunk
from a source that needs a live reader proof was never recognized as one: the
proof was not resolved, hydration ran without the caller's grants, and the
document was hidden from a member who can read it.

Until the keyword projection is filled, a page's unfilled rows now take their
source from the document, one primary-key read per row of the page after its
limit. The excluded-sources filter asks the document the same way, as the
vector leg already does, so a denied source's unfilled rows stop taking slots
on a rebuilt page. The filled path's statement is unchanged.
…e trigger now skips

The projection trigger no longer writes an ACL onto chunks the backfill has not
filled, so the ACL write tests can no longer see an unchanged write through
those rows. They now count the document writes that fire the fan-out trigger,
and expect a changed ACL on filled chunks only.
@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 22, 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 4 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 49f56c9 into staging Sep 22, 2026
34 checks passed
@waleedlatif1
waleedlatif1 deleted the fix/search-unfilled-row-source-from-document branch September 22, 2026 23:54

This branch was previously deployed

1 inactive deployment
Preview 574079f3 Deployed Sep 22, 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