Skip to content

db: raw reference-edge schema with lifecycle-safe grants (#83) - #90

Merged
IceRhymers merged 4 commits into
integration/knowledge-graph-reference-edgesfrom
feat/83-reference-edge-schema
Jul 23, 2026
Merged

db: raw reference-edge schema with lifecycle-safe grants (#83)#90
IceRhymers merged 4 commits into
integration/knowledge-graph-reference-edgesfrom
feat/83-reference-edge-schema

Conversation

@IceRhymers

Copy link
Copy Markdown
Owner

Part of #82 / umbrella #89.

Closes #83 — note: GitHub only auto-closes issues on a default-branch merge. This PR merges into integration/knowledge-graph-reference-edges, not master, so #83 must be closed manually once this lands on the integration branch (or tracked closed via #89 when the umbrella eventually merges to master).

What & why

Adds the reference_edges table (migration 0005) for raw, unresolved call/import edges — the storage primitive epic #82 needs before a writer (#84), a query-time resolver (#86), MCP tools (#87), or webui (#88) can exist. Scope is schema-only, exactly per the approved plan: no writer, no resolver, no MCP/webui changes, no INDEX_SEMANTICS_VERSION bump.

Design

  • New ReferenceEdge model (app/db/models.py) in Base.metadata (autogenerate-owned, unlike chunks): id (BigInteger PK), repo_id/file_id (FK → repos/files, both ON DELETE CASCADE), edge_kind (Text, CHECK IN ('call','import')), target_name (Text, NOT NULL), line (Integer, NOT NULL), enclosing_name/enclosing_kind/enclosing_start_line/enclosing_end_line (all nullable — NULL means module/top-level scope).
  • Deliberately NO foreign key to symbols (hard epic-[Epic] Knowledge graph: typed reference edges in Lakebase with agent-facing graph tools #82 rule): symbol ids churn on every per-file delete-and-reinsert; resolution from target_name to a concrete symbol happens at query time by name-join in a later child, not via a stored FK. Enforced by two independent tripwires: an ORM-metadata exact-FK-set check (tests/unit/test_reference_edge_model.py) and a source-level regex check on the migration file (tests/unit/test_migration_source.py::test_0005_no_symbol_fk).
  • Four indexes: btree target_name (resolver equality join), GIN trgm target_name (substring lookups, parity with ix_symbols_name_trgm), btree file_id (write-path + FK-cascade performance — Postgres doesn't auto-index FKs), btree (repo_id, edge_kind) (per-repo kind scans).
  • No new extension: pg_trgm already exists since migration 0001.
  • indexer/store.py changes are docstring-only: the four cascade-enumerating docstrings (index_repo step 4, _sweep_membership, reconcile_retired_branches step 3, reconcile_removed_repos step 1) now list reference_edges alongside symbols/chunks, keeping them truthful — the actual cascade is proven by the existing ON DELETE CASCADE FKs, no logic changed.
  • Grants: no code change to app/db/grants.py — the builders are schema-wide (GRANT ... ON ALL TABLES + ALTER DEFAULT PRIVILEGES), so they cover the new table automatically. New runbook docs/runbooks/reference-edges.md documents the ADP same-role/different-role semantics and the re-grant command for the different-identity case.

Acceptance-criteria mapping (issue #83)

# Criterion Where satisfied
1 Migration applies cleanly on local Postgres (CI integration path) and on Lakebase dev tests/integration/test_migrations.py shape/cascade/downgrade tests, run against a real Postgres 16 + pg_trgm container (see Testing below); Lakebase dev run — see Grant verification below (blocked, transparently)
2 App SP can SELECT and job SP can INSERT/DELETE on the new table on a fresh deploy, or the runbook documents the extra grant step test_reference_edges_adp_same_role_covers_new_table (positive proof, now also asserts the job role has no DDL) + test_reference_edges_adp_different_role_does_not_cover_new_table (negative proof) — both executed and passing against real Postgres; runbook documents the re-grant step
3 No cross-file/cross-repo symbol FK anywhere in the schema Model FKs are repos/files only; two independent tripwires (ORM + source-level) enforce this, both passing
4 EXPLAIN on the resolver join shape (target_name equality + trgm) confirms index usage on fixture data test_reference_edges_explain_resolver_join_uses_target_name_index + test_reference_edges_explain_ilike_uses_trgm_gin_index — both executed and passing against real Postgres

Testing (fresh output)

  • make lint — clean (ruff check + format + mypy app indexer webui).
  • make test — 901 passed, 204 deselected.
  • Integration — the repo's only integration gate is ci-lakebase.yml (ephemeral Lakebase branch, gated on CI_LAKEBASE_ENABLED). Locally, no lakebase_* extensions are available, so I built a fixture (migrated_edges_capable / _upgrade_edges_capable in test_migrations.py) that pre-seeds a stub chunks table before migration 0004 when those extensions are absent — the exact idempotency guard test_0004_guard_preserves_preexisting_chunks already exercises — so 0005 (pure pg_trgm, no Lakebase dependency) can still be applied and tested for real. On real Lakebase this same fixture takes the native upgrade head path instead.
    • Against a local Postgres 16 + pg_trgm container: all 7 new reference_edges-specific tests in test_migrations.py pass (shape/constraints, cascade, ADP same-role incl. job-role-has-no-DDL, ADP different-role, 2× EXPLAIN, downgrade/re-upgrade), plus the extended test_mark_and_sweep_removes_deleted_file cascade test in test_store.py (17/17 in that file).
    • Full local pytest -m "integration or e2e": 149 passed (baseline before this PR was 142; +7 new reference_edges tests, no regressions). The 8 pre-existing failures and 41 pre-existing errors are unchanged before/after this PR — they're all gated on lakebase_vector/lakebase_tokenizer extensions genuinely unavailable in this sandbox (e.g. test_reconcile.py's fixture unconditionally does CREATE EXTENSION lakebase_vector CASCADE), not something this PR introduced. These run for real on ci-lakebase.yml.
    • pytest --collect-only -m "integration or e2e": 204/1105 collected cleanly, confirming no syntax/import errors in the extended test_reconcile.py.

Grant verification (Lakebase dev)

Blocked, stated transparently: no code-search/codesearch Lakebase database instance exists in the connected Databricks workspace (databricks database list-database-instances lists only unrelated instances), so the has_table_privilege(...) dev-environment check from the plan could not be run. This is covered instead by:

  1. The CI proof (test_reference_edges_adp_same_role_covers_new_table / ..._different_role_does_not_cover_new_table), executed and passing locally against real Postgres — proves the underlying Postgres ADP semantics the grant-coupling claim depends on.
  2. The runbook (docs/runbooks/reference-edges.md) documenting the verification query and the unconditional re-grant fallback command.

An independent security review (fresh security-reviewer agent, mandated because this PR touches grants) rated this LOW risk, no blockers — see review notes below.

Review

Ran a fresh, independent code-reviewer + security-reviewer pass (parallel, separate context from implementation) against the full diff. Both returned APPROVE, zero blockers/majors. Three low-cost nits/informational findings were fixed in a follow-up commit:

  • Source-level no-symbols-FK tripwire made quote-agnostic (regex instead of a literal substring) so it survives formatter drift.
  • ReferenceEdge re-exported from app/db/__init__.py for symmetry with the other models.
  • Added a negative assertion that the job role's grants are DML-only (no TRUNCATE), matching build_job_grants' least-privilege intent.

Two informational, explicitly-out-of-scope notes were carried forward rather than fixed here: (a) the future #86 resolver's name-join must be repo-scoped to avoid cross-repo symbol disclosure (no resolver exists yet in this PR), and (b) the role-identifier regex in app/db/grants.py could be tightened as optional hardening (not required — Databricks SP client-ids are UUIDs, so it never triggers in practice).

Adds the reference_edges table (migration 0005) for raw, unresolved
call/import edges extracted per file. Deliberately no FK to symbols --
resolution to a concrete symbol happens at query time by name-join in a
later child of epic #82. FKs to repos/files only, both ON DELETE CASCADE,
so the existing sweep/reconcile cascade paths in indexer/store.py cover
the new table without any behavior change; their docstrings are updated
to say so truthfully. Grant builders in app/db/grants.py are schema-wide
and need no code change to cover the new table.
Unit: source-level tripwires for migration 0005 (revision chain, no
symbols FK, no app import, no stray CREATE EXTENSION) plus an ORM-metadata
tripwire (test_reference_edge_model.py) so a future models.py edit can't
silently reintroduce a symbols FK or loosen a NOT NULL column. Extends the
durable-core-tables tripwire in test_db_client.py.

Integration: adds migrated_edges_capable, a fixture that reaches migration
head on stock dev Postgres too (by pre-seeding a stub chunks table before
0004, the same idempotency guard test_0004_guard_preserves_preexisting_chunks
already exercises) so the reference_edges shape/cascade/EXPLAIN/downgrade
tests and both ADP same-role/different-role grant-lifecycle proofs run
without a live Lakebase branch. Extends test_reconcile.py and test_store.py
to seed reference_edges rows and assert they cascade through the existing
sweep/reconcile paths.
New docs/runbooks/reference-edges.md: schema summary, index-to-consumer
mapping, and the deploy/grant-coupling section (same shape as
multi-branch.md and semantic-enablement.md) with the ADP same-role/
different-role rule and the has_table_privilege verification query.
Links it from README's further-reading list. Updates app/db/AGENTS.md
and app/alembic/AGENTS.md key-file tables and the 0001->0005 chain.
…eEdge, assert job role has no DDL

Fresh code-reviewer and security-reviewer passes both returned APPROVE with
no blockers/majors; three low-cost nits/informational findings addressed:
- test_0005_no_symbol_fk now matches symbols.id via regex instead of a
  quote-literal substring, so it survives formatter drift.
- app/db/__init__.py re-exports ReferenceEdge alongside the other models.
- The ADP same-role grant test now also asserts the job role's grants are
  DML-only (TRUNCATE raises InsufficientPrivilege), matching
  build_job_grants' least-privilege intent.
@IceRhymers
IceRhymers merged commit f33ac6e into integration/knowledge-graph-reference-edges Jul 23, 2026
4 checks passed
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