From d05294d914a4be7b55253fa29281f6f8fce26c58 Mon Sep 17 00:00:00 2001 From: IceRhymers Date: Thu, 23 Jul 2026 13:55:20 -0700 Subject: [PATCH] indexer: emit typed Python call/import edges with enclosing attribution (#84) extract_file() extends the existing tree-sitter symbol walk in indexer/symbols.py to also emit call/import reference edges in the same single pass (indexer/languages.py's new EDGE_NODE_KINDS map, Python-only for now). Call targets resolve to the rightmost identifier of the callee; import targets are the full dotted path as written, alias-insensitive, with source-faithful relative-import and wildcard handling. Each edge attributes to the innermost named enclosing definition on the walk stack, computed in O(1) with no second walk. indexer/store.py's index_repo writes reference_edges exactly like symbols: an unconditional per-file delete followed by a bulk reinsert inside the same per-(repo, branch) transaction, so a file whose edges all vanish still sheds its stale rows. IndexCounts gains an edges count; indexer/job.py switches to extract_file and logs it. INDEX_SEMANTICS_VERSION bumps 2 -> 3 so every already-indexed branch re-indexes once to backfill reference_edges. --- app/db/AGENTS.md | 2 +- app/db/models.py | 6 +- docs/runbooks/reference-edges.md | 37 ++- indexer/AGENTS.md | 12 +- indexer/job.py | 9 +- indexer/languages.py | 41 +++- indexer/store.py | 49 +++- indexer/symbols.py | 235 +++++++++++++++++-- tests/integration/test_reconcile.py | 18 +- tests/integration/test_store.py | 191 +++++++++++++-- tests/integration/test_store_chunk_writer.py | 29 ++- tests/unit/AGENTS.md | 3 +- tests/unit/test_edges.py | 183 +++++++++++++++ tests/unit/test_job.py | 33 +-- tests/unit/test_job_redaction.py | 2 +- tests/unit/test_languages.py | 33 ++- tests/unit/test_store_chunk_writer.py | 23 +- 17 files changed, 787 insertions(+), 119 deletions(-) create mode 100644 tests/unit/test_edges.py diff --git a/app/db/AGENTS.md b/app/db/AGENTS.md index b0258dd..a1e3aa4 100644 --- a/app/db/AGENTS.md +++ b/app/db/AGENTS.md @@ -10,7 +10,7 @@ Database connectivity and schema truth for the code-search corpus. `client.py` i | File | Description | |------|-------------| | `client.py` | `create_db_engine()`: Lakebase-endpoint-wins-over-`PGHOST` dual-mode selection; Lakebase mode closes one `WorkspaceClient` over a `do_connect` handler that mints a fresh OAuth token as the password on every physical connect (never logged); server defaults `pool_size=5`, `pool_recycle=2700` (45 min, under the ~1h token TTL), `pool_pre_ping=True`; SDK import is lazy so the local path never touches it | -| `models.py` | ORM models + `INDEX_SEMANTICS_VERSION` (currently 2; bump on any indexing-meaning change — CI tripwires it). `File` is content-deduped per (repo_id, path, content_sha) with a `branches ARRAY(Text)` membership column; `RepoBranch` is the authoritative per-(repo, branch) CAS stamp; `repos`' own stamp columns and `files.commit` are deprecated/ambiguous — never add readers. `ReferenceEdge` (0005, epic #82) is a raw unresolved call/import edge, deliberately with NO FK to `symbols` (resolution happens at query time by name-join in a later child); FKs to `repos`/`files` only, both `ON DELETE CASCADE`. Declares the trgm + branches GIN indexes so autogenerate can't drift | +| `models.py` | ORM models + `INDEX_SEMANTICS_VERSION` (currently 3; bump on any indexing-meaning change — CI tripwires it). `File` is content-deduped per (repo_id, path, content_sha) with a `branches ARRAY(Text)` membership column; `RepoBranch` is the authoritative per-(repo, branch) CAS stamp; `repos`' own stamp columns and `files.commit` are deprecated/ambiguous — never add readers. `ReferenceEdge` (0005, epic #82) is a raw unresolved call/import edge, deliberately with NO FK to `symbols` (resolution happens at query time by name-join in a later child); FKs to `repos`/`files` only, both `ON DELETE CASCADE`. Declares the trgm + branches GIN indexes so autogenerate can't drift | | `semantic.py` | Standalone Core `Table` for `chunks` (BigInteger PK, `Vector(SEMANTIC_EMBEDDING_DIM)` embedding, generated `ts` tsvector, nullable `start_line`/`end_line`) in its own `semantic_metadata` — a typed description only; the real DDL is owned by migration `0004` | | `grants.py` | Pure SQL-string builders `build_app_grants` (read-only) / `build_job_grants` (CRUD + sequences, no DDL); identifiers validated against `^[A-Za-z0-9_-]+$` (1..63 chars) then psycopg-quoted. Execution lives in `scripts/migrate.py` | | `__init__.py` | Re-exports `Base`, `File`, `Repo`, `Symbol`, `create_db_engine` | diff --git a/app/db/models.py b/app/db/models.py index 37133e6..4364430 100644 --- a/app/db/models.py +++ b/app/db/models.py @@ -26,13 +26,17 @@ from sqlalchemy.dialects.postgresql import ARRAY from sqlalchemy.orm import DeclarativeBase, Mapped, mapped_column, relationship -INDEX_SEMANTICS_VERSION = 2 +INDEX_SEMANTICS_VERSION = 3 """Version of the indexing semantics the current code produces. 2: semantic search default-on -- every already-indexed branch must re-index once so ``chunks`` backfills (the skip seam compares ``(head_sha, INDEX_SEMANTICS_VERSION)``, and without a bump a repo already at HEAD would skip forever and never get chunks). +3: reference edges -- every already-indexed branch must re-index once so +``reference_edges`` backfills; without a bump a branch at HEAD would skip forever +and never get edges. + Bump this whenever the *meaning* of what gets written changes: any change to ``indexer/symbols.py``, to ``indexer/parse.py``'s chunking, or to ``indexer/languages.py``'s extraction contract. A bump forces every repo to diff --git a/docs/runbooks/reference-edges.md b/docs/runbooks/reference-edges.md index 3d7876f..838f976 100644 --- a/docs/runbooks/reference-edges.md +++ b/docs/runbooks/reference-edges.md @@ -27,15 +27,42 @@ joins through `files` with the same `coalesce(default_branch,'HEAD')` conjunct u everywhere else), and `files.commit` is documented-ambiguous under multi-branch dedup and must gain no new readers. -**This table is dormant until #84 ships a writer.** `0005` only creates the schema; no -code path inserts rows yet. `indexer/store.py`'s cascade-owning functions (`index_repo`'s -membership sweep, `reconcile_retired_branches`, `reconcile_removed_repos`) already -enumerate `reference_edges` alongside `symbols`/`chunks` in their docstrings and rely on -the same FK-cascade mechanism proven in §7.2 of the design doc and in +**#84 shipped the writer.** `indexer/symbols.py`'s `extract_file` walks each file's parse +tree once, emitting both `symbols` and `reference_edges` candidates from the same pass +(`indexer/languages.py`'s `EDGE_NODE_KINDS`, Python-only for now — the other six languages +land in #85). `indexer/store.py::index_repo` writes them exactly like `symbols`: an +unconditional per-file `DELETE` followed by a bulk reinsert, inside the same transaction as +the rest of that file's row, so a file whose edges all vanish still sheds its stale rows. +`indexer/store.py`'s cascade-owning functions (`index_repo`'s membership sweep, +`reconcile_retired_branches`, `reconcile_removed_repos`) already enumerate +`reference_edges` alongside `symbols`/`chunks` in their docstrings and rely on the same +FK-cascade mechanism proven in §7.2 of the design doc and in `tests/integration/test_reconcile.py` / `test_store.py` — no behavior change was needed to make the cascade correct, because both `repos -> reference_edges` and `files -> reference_edges` are `ON DELETE CASCADE` foreign keys. +**What gets extracted (Python, #84):** + +- **`call`** edges target the rightmost identifier of the callee: `f()` / `a.b.f()` / + `self.f()` all target `f`. Callees with no rightmost identifier (`xs[0]()`, the outer call + of `f()()`) are skipped — candidate-set semantics, not full resolution. +- **`import`** edges target the full dotted path as written, alias-insensitive: + `import a.b.c as d` targets `a.b.c`, not `d`. `from a.b import c, d as e` yields two edges + (`a.b.c`, `a.b.d`). Relative imports preserve source fidelity (`from . import x` -> + `.x`; `from ..p import q` -> `..p.q`). A wildcard `from a.b import *` yields one edge for + the module itself (`a.b`). +- **Enclosing attribution** is the innermost *named* definition on the walk stack when the + call/import node is visited (`None` = module/top-level scope) — a call in a class body + outside any method attributes to the class, not to `None`. +- Duplicate sites (the same target called twice on one line) are two rows by design; there + is no uniqueness constraint, and the query-time resolver (#86) ranks candidates. + +**Operational consequence of the `INDEX_SEMANTICS_VERSION` bump (2 -> 3, #84):** every +already-indexed branch's stored `(head_sha, index_semantics_version)` stamp now mismatches +the running code's version, so the *next* run of every branch is a full re-index (not a +skip) purely to backfill `reference_edges` — expected, one-time, and already how the `2` +bump behaved for `chunks`. + ## 2. Indexes | Index | Serves | diff --git a/indexer/AGENTS.md b/indexer/AGENTS.md index cdd44a2..eb5c590 100644 --- a/indexer/AGENTS.md +++ b/indexer/AGENTS.md @@ -4,7 +4,7 @@ # indexer ## Purpose -The serverless indexing job (`code-search-index`, wired as a `python_wheel_task` entry point in `pyproject.toml`). It reads the central `config.yaml` from the Databricks workspace, resolves it into a deduped list of GitHub repos (fail-fast on empty or oversized results, before any tarball is fetched or database connection opened), then fans repos out over a bounded thread pool. Per repo it resolves the default branch's immutable HEAD SHA, resolves configured branch globs into a concrete branch list, and — sequentially per branch — downloads the tarball by SHA over plain HTTPS (no git binary), extracts it safely, parses text files, extracts tree-sitter symbols, and writes everything in one atomic per-(repo, branch) transaction with content-SHA-deduped storage and a mark-and-sweep of stale branch membership. When semantic search is enabled, files are also chunked and embedded via `app.embed` — outside the transaction — and precomputed vectors are written through a `chunk_writer` seam. After every worker has joined, a post-fan-out checkpoint reconciles desired state — retiring stale branches and purging removed repos — but ONLY on a fully clean run (no failures, conflicts, or truncated branch discovery anywhere); a large repo-purge shrink is withheld as an incident signal rather than applied. The process exits non-zero if any branch fails, if reconciliation itself fails partway, or if a purge was withheld. +The serverless indexing job (`code-search-index`, wired as a `python_wheel_task` entry point in `pyproject.toml`). It reads the central `config.yaml` from the Databricks workspace, resolves it into a deduped list of GitHub repos (fail-fast on empty or oversized results, before any tarball is fetched or database connection opened), then fans repos out over a bounded thread pool. Per repo it resolves the default branch's immutable HEAD SHA, resolves configured branch globs into a concrete branch list, and — sequentially per branch — downloads the tarball by SHA over plain HTTPS (no git binary), extracts it safely, parses text files, extracts tree-sitter symbols and reference edges (typed call/import sites, Python-only for now), and writes everything in one atomic per-(repo, branch) transaction with content-SHA-deduped storage and a mark-and-sweep of stale branch membership. When semantic search is enabled, files are also chunked and embedded via `app.embed` — outside the transaction — and precomputed vectors are written through a `chunk_writer` seam. After every worker has joined, a post-fan-out checkpoint reconciles desired state — retiring stale branches and purging removed repos — but ONLY on a fully clean run (no failures, conflicts, or truncated branch discovery anywhere); a large repo-purge shrink is withheld as an incident signal rather than applied. The process exits non-zero if any branch fails, if reconciliation itself fails partway, or if a purge was withheld. ## Key Files | File | Description | @@ -15,12 +15,12 @@ The serverless indexing job (`code-search-index`, wired as a `python_wheel_task` | `fetch.py` | All GitHub HTTP: paginated org/user repo enumeration (`RepoMeta`), branch listing, `resolve_ref`/`resolve_branch_head` (branch name -> immutable SHA), streamed tarball download capped at `MAX_TARBALL_BYTES` (500 MB), safe extraction (`filter="data"`, bomb check) capped at `MAX_EXTRACTED_BYTES` (2 GB), `assert_disk_headroom` (2.5 GB per worker, both caps alive at once). `RateLimitError` is deliberately narrow: 429 always; 403 only with `Retry-After` or `X-RateLimit-Remaining: 0` — other 403s are permission failures. | | `hashing.py` | `content_sha`: canonical SHA-256 hex of content (`None` -> empty string). Single source of truth for `files.content_sha`; must stay byte-identical to the `0003` migration's SQL backfill forever or cross-branch dedup silently breaks (`tests/integration/test_content_sha_parity.py` is the gate). | | `job.py` | Entry point and orchestration: `main()`/`run()`, `ThreadPoolExecutor` sized by `effective_workers` (unit of work = one repo, all branches sequential), batched `repo_branches` stamp read, per-branch skip-if-unchanged, per-branch `BranchOutcome` classification (`indexed`/`skipped`/`conflict`/`failed`), semantic precompute (`_precompute_chunk_writer`), `ContextVar`-based `[repo]` log attribution. Returns 1 if any branch failed; conflicts self-heal and do not fail the run. **Reconciliation checkpoint**: after every worker joins, `_decide_reconciliation` gates on zero failures/conflicts, every repo accounted for, and every repo's `discovery_complete`; on a pass, `_reconcile` reuses the pre-fan-out stamp snapshot to compute each repo's retired branches and the run's full desired repo set, then calls the injected `reconcile_retired_fn`/`reconcile_removed_fn` seams (default: the real `indexer.store` primitives) on one post-fan-out connection — retired branches first, repo purge second. `MAX_PURGE_SHRINK_FRACTION = 0.5` (hardcoded, no config knob) withholds ONLY the purge, as a logged incident signal, if it would remove a strict majority of currently stored repos; retired-branch cleanup on survivors still applies. `ReconcileProgress.committed_any` drives an honest "partially reconciled" vs "left stale" failure message — never the raw exception, only its phase/repo/type name. Never called from worker-thread code (`_index_one*`) — a source-level tripwire test enforces it. | -| `languages.py` | Single source of truth shared by `parse.py` and `symbols.py`: `EXT_TO_LANG` (8 extensions -> tree-sitter language names), `SYMBOL_KINDS` (node type -> symbol kind per language), `MAX_FILE_BYTES` (1 MB), `SEMANTIC_CHUNK_MAX_CHARS` (2000, ~4 chars/token), and the frozen dataclasses `ParsedFile`, `Chunk`, `ExtractedSymbol`, `IndexCounts`. | +| `languages.py` | Single source of truth shared by `parse.py` and `symbols.py`: `EXT_TO_LANG` (8 extensions -> tree-sitter language names), `SYMBOL_KINDS` (node type -> symbol kind per language), `EDGE_NODE_KINDS` (node type -> reference-edge kind per language, Python-only until #85), `MAX_FILE_BYTES` (1 MB), `SEMANTIC_CHUNK_MAX_CHARS` (2000, ~4 chars/token), and the frozen dataclasses `ParsedFile`, `Chunk`, `ExtractedSymbol`, `ExtractedEdge`, `FileExtraction`, `IndexCounts` (now carries `edges`). | | `parse.py` | `iter_source_files`: walk an extracted tree, yield every text file (unknown extensions kept with `lang=None`, since grep runs over all files); skips `.git/`, symlinks, files > `MAX_FILE_BYTES` (stat before read), NUL-sniffed binaries, and UTF-8 decode failures; strips surviving NULs (Postgres `text` rejects them). `iter_chunks`: deterministic line-aligned chunking, no overlap, no mid-line splits, 1-based inclusive line ranges. | | `repo_config.py` | Pydantic schema for `config.yaml` (`RepoConfig` / `GitHubConnection` / `ExcludeRules`), `normalize_repo` (URL/SSH/bare -> canonical `org/repo`, GitHub hosts only), `parse_config` (pure) vs `read_workspace_config` (SDK I/O, wraps every failure in `ConfigError` with HTTP status so 404 "never synced" stays distinguishable from 403 "no permission"), and `effective_workers` (the semantic clamp). A connection with no `orgs`/`users`/`repos` fails validation. `index_concurrency`: 1-8, default 4 — a disk bound (2.5 GB peak per worker), not a CPU one. `semantic_max_chunks_per_repo`: optional top-level `{"org/repo": N}` map overriding `app.config.Settings.semantic_max_chunks_per_repo` per repo; keys canonicalised via `normalize_repo` and rejected on collision post-casefold, values `>= 1`. Deliberately import-light: pydantic + PyYAML + stdlib only. | | `resolve.py` | `resolve_repos`: enumerate org/user selectors, apply `ExcludeRules` to enumerated repos only (explicit `repos` entries always win, unfiltered), dedup case-insensitively keeping first-seen spelling, union each connection's `branches:` globs into per-repo `RepoEntry.branch_globs`, then fail fast: `EmptyConfigError` on zero repos (indexing nothing must not exit 0), `RepoCeilingError` above `MAX_REPOS` (500, overridable via `--max_repos`). Also matches `RepoConfig.semantic_max_chunks_per_repo` case-insensitively onto each resolved repo's `RepoEntry.semantic_max_chunks` (`None` if unmatched); an override key matching no resolved repo logs a WARNING (typo guard). Enumerator params are a test seam only, not provider dispatch. | -| `store.py` | `index_repo`: the single atomic unit of work for one (repo, branch) inside `with conn.begin():` — repos upsert, `repo_branches` CAS-baseline read, per-file array-union upsert on `(repo_id, path, content_sha)` with branch-membership union, delete-and-reinsert symbols, optional `chunk_writer` call, membership sweep (strip this branch from unseen rows, delete rows with empty `branches`; skipped with a WARNING on an empty seen-set), then CAS stamp (raises `StaleIndexError` on mismatch, rolling everything back). Pure DML, no TEMP tables (job role has no TEMP privilege on Lakebase). `reconcile_retired_branches` and `reconcile_removed_repos`: the desired-state storage primitives `job.py`'s reconciliation checkpoint calls — each its own `conn.begin()`, repo-scoped membership subtraction (never delete-by-path) and an exact-match repo purge respectively; neither decides WHAT is retired/desired, only applies a caller-supplied set (see `job.py`'s row). | -| `symbols.py` | `extract_symbols`: tree-sitter parse via `tree_sitter_language_pack`, full-tree walk (nested definitions captured), named nodes only, kinds from `SYMBOL_KINDS`, 1-based lines. Parser cache is per-thread (`threading.local`) as insurance against a future GIL-releasing `parse()`. | +| `store.py` | `index_repo`: the single atomic unit of work for one (repo, branch) inside `with conn.begin():` — repos upsert, `repo_branches` CAS-baseline read, per-file array-union upsert on `(repo_id, path, content_sha)` with branch-membership union, delete-and-reinsert `symbols` AND `reference_edges` (both keyed only by `file_id`, no natural key), optional `chunk_writer` call, membership sweep (strip this branch from unseen rows, delete rows with empty `branches`; skipped with a WARNING on an empty seen-set), then CAS stamp (raises `StaleIndexError` on mismatch, rolling everything back). `items` is `Iterable[tuple[ParsedFile, FileExtraction]]`; the `reference_edges` delete runs unconditionally, even when a file's `FileExtraction.edges` is empty, so stale rows never survive a re-index. Pure DML, no TEMP tables (job role has no TEMP privilege on Lakebase). `reconcile_retired_branches` and `reconcile_removed_repos`: the desired-state storage primitives `job.py`'s reconciliation checkpoint calls — each its own `conn.begin()`, repo-scoped membership subtraction (never delete-by-path) and an exact-match repo purge respectively; neither decides WHAT is retired/desired, only applies a caller-supplied set (see `job.py`'s row). | +| `symbols.py` | `extract_file` (#84): one tree-sitter parse, one full-tree walk, emitting both `symbols` and `reference_edges` candidates (`FileExtraction`) in the same pass — nested definitions captured, named nodes only for symbols, kinds from `SYMBOL_KINDS`/`EDGE_NODE_KINDS`, 1-based lines. Stack entries carry the innermost named enclosing definition so edges attribute to it in O(1) with no second walk. `extract_symbols` is a thin wrapper (`extract_file(pf).symbols`) kept for the existing call sites. Python-only edge helpers (`_python_call_target`, `_python_import_edges`) are the seam #85 generalizes to other languages. Parser cache is per-thread (`threading.local`) as insurance against a future GIL-releasing `parse()`. | ## For AI Agents @@ -38,12 +38,12 @@ The serverless indexing job (`code-search-index`, wired as a `python_wheel_task` - **Import discipline**: `repo_config.py` stays import-light (no httpx/SQLAlchemy/SDK at module level); `normalize_repo` is imported from `repo_config`, never from `job` (import cycle); `app.embed`'s databricks-sdk dependency is only imported when semantic is on. ### Testing Requirements -- `make test` (`pytest -m "unit or observability"`, no external deps): `tests/unit/test_branches.py`, `test_chunk_store.py`, `test_chunking.py`, `test_fetch.py`, `test_job.py`, `test_job_redaction.py`, `test_languages.py`, `test_parse.py`, `test_repo_config.py`, `test_resolve.py`, `test_symbols.py`, `test_store_chunk_writer.py`, `test_semantics_version_tripwire.py`. +- `make test` (`pytest -m "unit or observability"`, no external deps): `tests/unit/test_branches.py`, `test_chunk_store.py`, `test_chunking.py`, `test_edges.py`, `test_fetch.py`, `test_job.py`, `test_job_redaction.py`, `test_languages.py`, `test_parse.py`, `test_repo_config.py`, `test_resolve.py`, `test_symbols.py`, `test_store_chunk_writer.py`, `test_semantics_version_tripwire.py`. - `make test-integration` (needs Postgres): `tests/integration/test_store.py`, `test_store_chunk_writer.py`, `test_content_sha_parity.py` (the hard gate on `hashing.py` vs the migration backfill), `test_reconcile.py` (the `reconcile_retired_branches`/`reconcile_removed_repos` storage primitives directly), and `test_job_reconcile.py` (#59: the real `run()` end to end — real primitives, real engine, only GitHub HTTP faked). - Schema tests must stay fast: changes to `repo_config.py` must not add heavy imports. ### Common Patterns -- Frozen dataclasses as data carriers (`ParsedFile`, `Chunk`, `ExtractedSymbol`, `IndexCounts`, `RepoMeta`, `RepoEntry`, `BranchOutcome`); shared vocabulary lives in `languages.py` so `parse` and `symbols` can never disagree. +- Frozen dataclasses as data carriers (`ParsedFile`, `Chunk`, `ExtractedSymbol`, `ExtractedEdge`, `FileExtraction`, `IndexCounts`, `RepoMeta`, `RepoEntry`, `BranchOutcome`); shared vocabulary lives in `languages.py` so `parse` and `symbols` can never disagree. - `pg_insert(...).on_conflict_do_update(...).returning(...)` with a no-op `SET` on conflict — `DO NOTHING ... RETURNING` returns no row on conflict and would break the id bootstrap. - Delete-and-reinsert for child rows with no natural key (symbols, chunks). - `fnmatchcase`, never `fnmatch`: plain globs must behave identically on every platform. diff --git a/indexer/job.py b/indexer/job.py index f1510fb..83853be 100644 --- a/indexer/job.py +++ b/indexer/job.py @@ -130,7 +130,7 @@ reconcile_removed_repos, reconcile_retired_branches, ) -from indexer.symbols import extract_symbols +from indexer.symbols import extract_file logger = logging.getLogger("indexer.job") @@ -467,11 +467,12 @@ def run( ok += 1 assert outcome.counts is not None logger.info( - "indexed %s@%s: files=%d symbols=%d swept=%d", + "indexed %s@%s: files=%d symbols=%d edges=%d swept=%d", entry.name, outcome.branch, outcome.counts.files, outcome.counts.symbols, + outcome.counts.edges, outcome.counts.swept, ) @@ -1017,10 +1018,10 @@ def _index_one_branch( exc_info=True, ) chunk_writer = None - items = ((pf, extract_symbols(pf)) for pf in files) + items = ((pf, extract_file(pf)) for pf in files) else: # Lazy generator: files stream through the open transaction (bounded memory). - items = ((pf, extract_symbols(pf)) for pf in iter_source_files(root)) + items = ((pf, extract_file(pf)) for pf in iter_source_files(root)) with engine.connect() as conn: counts = index_fn( diff --git a/indexer/languages.py b/indexer/languages.py index 61f67b0..49fbc7d 100644 --- a/indexer/languages.py +++ b/indexer/languages.py @@ -3,7 +3,10 @@ Both :mod:`indexer.parse` (extension -> language) and :mod:`indexer.symbols` (language -> tree-sitter node-type -> symbol kind) import from here so they can never disagree on language names. Language values MUST be valid -``tree_sitter_language_pack`` parser names. +``tree_sitter_language_pack`` parser names. ``EDGE_NODE_KINDS`` maps, per +language, tree-sitter node ``.type`` -> reference-edge kind (``call``/``import``); +a language absent from the map yields zero edges (Python only for #84; #85 +adds the rest). """ from __future__ import annotations @@ -73,6 +76,18 @@ }, } +# Per language: tree-sitter node ``.type`` -> reference-edge kind stored in +# ``reference_edges``. Every value MUST be within the DB CHECK set +# (``ReferenceEdge.__table__``'s ``ck_reference_edges_edge_kind``), enforced by +# a unit test. Python only for #84 -- #85 adds the other six languages. +EDGE_NODE_KINDS: dict[str, dict[str, str]] = { + "python": { + "call": "call", + "import_statement": "import", + "import_from_statement": "import", + }, +} + @dataclass(frozen=True) class ParsedFile: @@ -104,6 +119,29 @@ class ExtractedSymbol: end_line: int +@dataclass(frozen=True) +class ExtractedEdge: + """A raw (unresolved) call/import reference site extracted from a source file. + + ``target`` is a candidate name/dotted-path, not a resolved symbol id (the + epic #82 rule: resolution happens at query time by name-join). ``enclosing`` + is the innermost NAMED enclosing definition; ``None`` means module scope. + """ + + kind: str # 'call' | 'import' -- must stay within the reference_edges DB CHECK set + target: str + line: int # 1-based + enclosing: ExtractedSymbol | None + + +@dataclass(frozen=True) +class FileExtraction: + """The one-walk result of parsing a file: its symbols and its reference edges.""" + + symbols: list[ExtractedSymbol] + edges: list[ExtractedEdge] + + @dataclass(frozen=True) class IndexCounts: """Row-count summary returned by ``index_repo`` for one repo's run.""" @@ -111,3 +149,4 @@ class IndexCounts: files: int symbols: int swept: int + edges: int diff --git a/indexer/store.py b/indexer/store.py index 4919f0c..f85f0df 100644 --- a/indexer/store.py +++ b/indexer/store.py @@ -26,9 +26,9 @@ from sqlalchemy import Connection, delete, func, text, update from sqlalchemy.dialects.postgresql import insert as pg_insert -from app.db.models import INDEX_SEMANTICS_VERSION, File, Repo, RepoBranch, Symbol +from app.db.models import INDEX_SEMANTICS_VERSION, File, ReferenceEdge, Repo, RepoBranch, Symbol from indexer.hashing import content_sha -from indexer.languages import ExtractedSymbol, IndexCounts, ParsedFile +from indexer.languages import FileExtraction, IndexCounts, ParsedFile logger = logging.getLogger("indexer.store") @@ -76,7 +76,7 @@ def index_repo( branch: str, is_default: bool, head_sha: str, - items: Iterable[tuple[ParsedFile, list[ExtractedSymbol]]], + items: Iterable[tuple[ParsedFile, FileExtraction]], chunk_writer: ChunkWriter | None = None, ) -> IndexCounts: """Upsert one ``(repo, branch)``'s files/symbols and sweep this branch's stale membership. @@ -98,10 +98,10 @@ def index_repo( whose content already exists under another branch gets THIS branch unioned into its ``branches`` array (one row, shared content); a file whose content differs from every existing version gets its own row. Then - delete-and-reinsert its ``symbols`` (no natural key), then call - ``chunk_writer`` (if given) so chunk writes commit/roll back with the - rest of that file's row. Each processed file's ``(path, content_sha)`` is - collected into this branch's seen-set. + delete-and-reinsert its ``symbols`` and ``reference_edges`` (neither has a + natural key), then call ``chunk_writer`` (if given) so chunk writes + commit/roll back with the rest of that file's row. Each processed file's + ``(path, content_sha)`` is collected into this branch's seen-set. 4. Membership sweep, keyed on THIS branch's seen-set (never on ``commit``, which is ambiguous under dedup): strip ``branch`` from any row's ``branches`` array that is not in the seen-set, then delete any row left @@ -121,6 +121,7 @@ def index_repo( """ file_count = 0 symbol_count = 0 + edge_count = 0 seen_paths: list[str] = [] seen_shas: list[str] = [] @@ -157,7 +158,7 @@ def index_repo( ) baseline_commit, baseline_version = conn.execute(branch_stmt).one() - for pf, syms in items: + for pf, ex in items: sha = content_sha(pf.content) file_stmt = ( pg_insert(File) @@ -197,7 +198,7 @@ def index_repo( seen_shas.append(sha) conn.execute(delete(Symbol).where(Symbol.file_id == file_id)) - if syms: + if ex.symbols: conn.execute( pg_insert(Symbol), [ @@ -209,10 +210,34 @@ def index_repo( "start_line": s.start_line, "end_line": s.end_line, } - for s in syms + for s in ex.symbols ], ) - symbol_count += len(syms) + symbol_count += len(ex.symbols) + + # UNCONDITIONAL, same as the symbols delete above: a file whose edges + # all vanish (e.g. every call/import site removed) must shed its stale + # rows even when this run's ex.edges is empty. + conn.execute(delete(ReferenceEdge).where(ReferenceEdge.file_id == file_id)) + if ex.edges: + conn.execute( + pg_insert(ReferenceEdge), + [ + { + "file_id": file_id, + "repo_id": repo_id, + "edge_kind": e.kind, + "target_name": e.target, + "line": e.line, + "enclosing_name": e.enclosing.name if e.enclosing else None, + "enclosing_kind": e.enclosing.kind if e.enclosing else None, + "enclosing_start_line": e.enclosing.start_line if e.enclosing else None, + "enclosing_end_line": e.enclosing.end_line if e.enclosing else None, + } + for e in ex.edges + ], + ) + edge_count += len(ex.edges) if chunk_writer is not None: chunk_writer(conn, repo_id, file_id, pf) @@ -236,7 +261,7 @@ def index_repo( baseline_version=baseline_version, ) - return IndexCounts(files=file_count, symbols=symbol_count, swept=swept) + return IndexCounts(files=file_count, symbols=symbol_count, swept=swept, edges=edge_count) def _sweep_membership( diff --git a/indexer/symbols.py b/indexer/symbols.py index e7c6ee4..a50d466 100644 --- a/indexer/symbols.py +++ b/indexer/symbols.py @@ -20,10 +20,40 @@ from tree_sitter_language_pack import get_parser -from indexer.languages import SYMBOL_KINDS, ExtractedSymbol, ParsedFile +from indexer.languages import ( + EDGE_NODE_KINDS, + SYMBOL_KINDS, + ExtractedEdge, + ExtractedSymbol, + FileExtraction, + ParsedFile, +) _PARSER_CACHE = threading.local() +# Per-language SYMBOL_KINDS + EDGE_NODE_KINDS merged into one ``node.type -> (tag, +# value)`` map, built once per language and cached here. The two source maps never +# share a node type (a definition node is never also a call/import node), so this +# is a lossless merge -- and it turns the hot walk's two dict lookups per node +# (one symbol-map miss, one edge-map miss, for every ordinary node) into one. +_COMBINED_CACHE: dict[str, dict[str, tuple[str, str]]] = {} + + +def _combined_kinds(lang: str) -> dict[str, tuple[str, str]] | None: + combined = _COMBINED_CACHE.get(lang) + if combined is not None: + return combined + kind_map = SYMBOL_KINDS.get(lang) + if kind_map is None: + return None + combined = {node_type: ("symbol", kind) for node_type, kind in kind_map.items()} + combined.update( + (node_type, ("edge", edge_kind)) + for node_type, edge_kind in EDGE_NODE_KINDS.get(lang, {}).items() + ) + _COMBINED_CACHE[lang] = combined + return combined + def _parser_for(lang: str) -> Any: cache: dict[str, Any] | None = getattr(_PARSER_CACHE, "parsers", None) @@ -35,37 +65,196 @@ def _parser_for(lang: str) -> Any: return parser -def extract_symbols(pf: ParsedFile) -> list[ExtractedSymbol]: - """Return the named symbols in ``pf``; ``[]`` for files with no kind map. +def extract_file(pf: ParsedFile) -> FileExtraction: + """Return the named symbols and reference edges in ``pf``, from one parse and one walk. + + Walks the whole parse tree (so nested definitions -- a method inside a class -- + are captured) exactly once, emitting both symbols and edges as it goes; a file + whose language has no kind map short-circuits before parsing. Anonymous + definition nodes (no ``name`` field) are skipped for symbols but stay + transparent for edge attribution: their children inherit the enclosing symbol + they would otherwise have replaced. Edges attribute to the innermost NAMED + enclosing definition on the stack at the time the call/import node is visited; + ``None`` means module/top-level scope. Line numbers are 1-based. - Walks the whole parse tree (so nested definitions — a method inside a class — - are captured). Anonymous nodes (no ``name`` field) are skipped. Line numbers - are 1-based. + Two parallel stacks (node, enclosing-symbol) rather than one stack of pairs -- + pushing a same-enclosing child run via ``[enclosing] * len(children)`` is a + single C-level list replication instead of N per-child tuple allocations, + measurably cheaper for the common case (most nodes don't change the enclosing). """ - if pf.lang is None: - return [] - kind_map = SYMBOL_KINDS.get(pf.lang) - if kind_map is None: - return [] + lang = pf.lang + if lang is None: + return FileExtraction(symbols=[], edges=[]) + combined = _combined_kinds(lang) + if combined is None: + return FileExtraction(symbols=[], edges=[]) - tree = _parser_for(pf.lang).parse(pf.content.encode("utf-8")) + tree = _parser_for(lang).parse(pf.content.encode("utf-8")) symbols: list[ExtractedSymbol] = [] + edges: list[ExtractedEdge] = [] - cursor_stack = [tree.root_node] - while cursor_stack: - node = cursor_stack.pop() - kind = kind_map.get(node.type) - if kind is not None: - name_node = node.child_by_field_name("name") - if name_node is not None and name_node.text is not None: - symbols.append( - ExtractedSymbol( + node_stack: list[Any] = [tree.root_node] + enclosing_stack: list[ExtractedSymbol | None] = [None] + while node_stack: + node = node_stack.pop() + enclosing = enclosing_stack.pop() + child_enclosing = enclosing + + tag_kind = combined.get(node.type) + if tag_kind is not None: + tag, kind = tag_kind + if tag == "symbol": + name_node = node.child_by_field_name("name") + if name_node is not None and name_node.text is not None: + symbol = ExtractedSymbol( name=name_node.text.decode("utf-8"), kind=kind, start_line=node.start_point[0] + 1, end_line=node.end_point[0] + 1, ) + symbols.append(symbol) + child_enclosing = symbol + elif kind == "call": + edge = _python_call_edge(node, enclosing) + if edge is not None: + edges.append(edge) + else: # kind == "import" + edges.extend(_python_import_edges(node, enclosing)) + + children = node.children + if children: + node_stack.extend(reversed(children)) + enclosing_stack.extend([child_enclosing] * len(children)) + + return FileExtraction(symbols=symbols, edges=edges) + + +def extract_symbols(pf: ParsedFile) -> list[ExtractedSymbol]: + """Return the named symbols in ``pf``; ``[]`` for files with no kind map. + + Thin wrapper over :func:`extract_file` kept for the existing unit-test + surface and any external callers that only need symbols. + """ + return extract_file(pf).symbols + + +def _python_call_target(node: Any) -> str | None: + """Rightmost identifier of a ``call`` node's callee, or ``None`` for candidates with none. + + ``f(...)`` -> ``f``; ``a.b.f(...)``/``self.f(...)`` -> ``f`` (the grammar's + ``attribute`` field on an ``attribute`` node is always the rightmost + identifier, so no manual recursion is needed). Callees with no rightmost + identifier -- ``xs[0]()``, the outer call of ``f()()`` -- are skipped. + """ + func = node.child_by_field_name("function") + if func is None: + return None + if func.type == "identifier": + return func.text.decode("utf-8") if func.text is not None else None + if func.type == "attribute": + attr = func.child_by_field_name("attribute") + if attr is not None and attr.text is not None: + return attr.text.decode("utf-8") + return None + return None + + +def _python_call_edge(node: Any, enclosing: ExtractedSymbol | None) -> ExtractedEdge | None: + target = _python_call_target(node) + if target is None: + return None + return ExtractedEdge( + kind="call", target=target, line=node.start_point[0] + 1, enclosing=enclosing + ) + + +def _python_import_name(node: Any) -> tuple[str | None, Any]: + """Dotted-path text and line-anchor node for one ``name``-field child of an import. + + ``dotted_name`` -> its own text (``import a.b.c`` -> ``a.b.c``). ``aliased_import`` + -> its inner ``name`` field's text, ignoring the alias (``import a.b.c as d`` -> + ``a.b.c``; the alias is a local binding, not the target). + """ + if node.type == "aliased_import": + inner = node.child_by_field_name("name") + if inner is not None and inner.text is not None: + return inner.text.decode("utf-8"), node + return None, node + if node.text is not None: + return node.text.decode("utf-8"), node + return None, node + + +def _python_join_module(module_prefix: str, name: str) -> str: + """Join a ``from``-import's module path to one imported name (D5's join rule). + + Pure-dots relative modules (module text ending in ``.``, e.g. ``from . import x`` + -> ``.``) concatenate directly (-> ``.x``); anything else (``a.b``, ``..p``) joins + with a literal dot (-> ``a.b.c``, ``..p.q``). + """ + if not module_prefix: + return name + if module_prefix.endswith("."): + return f"{module_prefix}{name}" + return f"{module_prefix}.{name}" + + +def _python_import_edges(node: Any, enclosing: ExtractedSymbol | None) -> list[ExtractedEdge]: + """Edges for one ``import_statement``/``import_from_statement`` node (D5). + + ``import a.b.c, d`` -> one edge per ``name``-field child, each the full dotted + path as written (alias-insensitive). ``from a.b import c, d as e`` -> the module + path joined to each *original* imported name. ``from a.b import *`` -> one edge + for the module path itself, anchored at the ``wildcard_import`` node's line. + Per-name edges take the name node's own start line (correct for multi-line + parenthesized imports). + """ + edges: list[ExtractedEdge] = [] + if node.type == "import_statement": + for name_node in node.children_by_field_name("name"): + target, anchor = _python_import_name(name_node) + if target is not None: + edges.append( + ExtractedEdge( + kind="import", + target=target, + line=anchor.start_point[0] + 1, + enclosing=enclosing, + ) ) - cursor_stack.extend(reversed(node.children)) + return edges - return symbols + # import_from_statement + module_node = node.child_by_field_name("module_name") + module_prefix = ( + module_node.text.decode("utf-8") + if module_node is not None and module_node.text is not None + else "" + ) + name_nodes = node.children_by_field_name("name") + if name_nodes: + for name_node in name_nodes: + bare, anchor = _python_import_name(name_node) + if bare is not None: + edges.append( + ExtractedEdge( + kind="import", + target=_python_join_module(module_prefix, bare), + line=anchor.start_point[0] + 1, + enclosing=enclosing, + ) + ) + return edges + + if module_prefix: + wildcard = next((c for c in node.children if c.type == "wildcard_import"), None) + if wildcard is not None: + edges.append( + ExtractedEdge( + kind="import", + target=module_prefix, + line=wildcard.start_point[0] + 1, + enclosing=enclosing, + ) + ) + return edges diff --git a/tests/integration/test_reconcile.py b/tests/integration/test_reconcile.py index d377e43..4e98cbd 100644 --- a/tests/integration/test_reconcile.py +++ b/tests/integration/test_reconcile.py @@ -25,7 +25,7 @@ from app.db.grants import build_job_grants from app.db.models import Base from indexer.chunk_store import write_chunks -from indexer.languages import ExtractedSymbol, ParsedFile +from indexer.languages import ExtractedSymbol, FileExtraction, ParsedFile from indexer.store import ( ReconcileCounts, index_repo, @@ -80,8 +80,11 @@ def _pf(path: str, content: str) -> ParsedFile: def _items( *specs: tuple[str, str, list[ExtractedSymbol]], -) -> list[tuple[ParsedFile, list[ExtractedSymbol]]]: - return [(_pf(path, content), syms) for path, content, syms in specs] +) -> list[tuple[ParsedFile, FileExtraction]]: + return [ + (_pf(path, content), FileExtraction(symbols=syms, edges=[])) + for path, content, syms in specs + ] MAIN = ("main.py", "def f():\n return 1\n", [ExtractedSymbol("f", "function", 1, 2)]) @@ -148,7 +151,14 @@ def _cfg() -> Settings: def _seed_reference_edge( conn: Connection, *, repo_id: int, file_id: int, target_name: str = "target_fn" ) -> None: - """Seed one raw reference edge row (indexer.store has no writer yet -- #84).""" + """Seed one raw reference edge row directly. + + These reconcile tests exercise the storage primitives (retirement/purge + cascades) in isolation from the real extractor, which landed in #84 + (``indexer.symbols.extract_file`` / ``indexer.store.index_repo``'s edge + writer) -- seeding a row by hand keeps this module focused on + ``reconcile_retired_branches``/``reconcile_removed_repos`` alone. + """ conn.execute( text( "INSERT INTO reference_edges (repo_id, file_id, edge_kind, target_name, line) " diff --git a/tests/integration/test_store.py b/tests/integration/test_store.py index 7aff396..194fef5 100644 --- a/tests/integration/test_store.py +++ b/tests/integration/test_store.py @@ -24,7 +24,13 @@ from app.db.client import create_db_engine from app.db.grants import build_job_grants from app.db.models import INDEX_SEMANTICS_VERSION, Base -from indexer.languages import ExtractedSymbol, IndexCounts, ParsedFile +from indexer.languages import ( + ExtractedEdge, + ExtractedSymbol, + FileExtraction, + IndexCounts, + ParsedFile, +) from indexer.store import StaleIndexError, _stamp_repo_branch, index_repo SCHEMA = "test_store" @@ -58,9 +64,20 @@ def _pf(path: str, content: str) -> ParsedFile: def _items( - *specs: tuple[str, str, list[ExtractedSymbol]], -) -> list[tuple[ParsedFile, list[ExtractedSymbol]]]: - return [(_pf(path, content), syms) for path, content, syms in specs] + *specs: ( + tuple[str, str, list[ExtractedSymbol]] + | tuple[str, str, list[ExtractedSymbol], list[ExtractedEdge]] + ), +) -> list[tuple[ParsedFile, FileExtraction]]: + result: list[tuple[ParsedFile, FileExtraction]] = [] + for spec in specs: + if len(spec) == 3: + path, content, syms = spec + edges: list[ExtractedEdge] = [] + else: + path, content, syms, edges = spec + result.append((_pf(path, content), FileExtraction(symbols=syms, edges=edges))) + return result MAIN = ("main.py", "def f():\n return 1\n", [ExtractedSymbol("f", "function", 1, 2)]) @@ -79,7 +96,7 @@ def _index_default( *, name: str, head_sha: str, - items: Iterable[tuple[ParsedFile, list[ExtractedSymbol]]], + items: Iterable[tuple[ParsedFile, FileExtraction]], ) -> IndexCounts: """Shorthand for the pre-multi-branch call shape: one default branch, "main".""" return index_repo( @@ -92,7 +109,7 @@ def test_first_run_populates_and_stamps_commit(conn: Connection) -> None: counts = _index_default( conn, name="acme/widgets", head_sha="sha_first", items=_items(MAIN, UTIL) ) - assert counts == IndexCounts(files=2, symbols=2, swept=0) + assert counts == IndexCounts(files=2, symbols=2, swept=0, edges=0) assert _count(conn, "repos") == 1 assert _count(conn, "files") == 2 @@ -122,7 +139,7 @@ def test_rerun_is_idempotent(conn: Connection) -> None: counts = _index_default( conn, name="acme/widgets", head_sha="sha_first", items=_items(MAIN, UTIL) ) - assert counts == IndexCounts(files=2, symbols=2, swept=0) + assert counts == IndexCounts(files=2, symbols=2, swept=0, edges=0) assert _count(conn, "repos") == 1 assert _count(conn, "files") == 2 assert _count(conn, "symbols") == 2 @@ -130,21 +147,20 @@ def test_rerun_is_idempotent(conn: Connection) -> None: @pytest.mark.integration def test_mark_and_sweep_removes_deleted_file(conn: Connection) -> None: - _index_default(conn, name="acme/widgets", head_sha="sha_first", items=_items(MAIN, UTIL)) - repo_id = conn.execute(text("SELECT id FROM repos WHERE name = 'acme/widgets'")).scalar_one() + # util.py has a real call site so the writer produces a reference_edges row + # -- proves the sweep's FK cascade reaches this table too. + util_symbol = ExtractedSymbol("g", "function", 1, 3) + util_with_edge = ( + "util.py", + "def g():\n helper()\n return 2\n", + [util_symbol], + [ExtractedEdge(kind="call", target="helper", line=2, enclosing=util_symbol)], + ) + _index_default( + conn, name="acme/widgets", head_sha="sha_first", items=_items(MAIN, util_with_edge) + ) removed_file_id = conn.execute(text("SELECT id FROM files WHERE path = 'util.py'")).scalar_one() assert _count(conn, "symbols", f"file_id = {removed_file_id}") == 1 - - # index_repo has no reference_edges writer yet (#84); seed one directly to - # prove the sweep's FK cascade reaches this table too. - conn.execute( - text( - "INSERT INTO reference_edges (repo_id, file_id, edge_kind, target_name, line) " - "VALUES (:r, :f, 'call', 'target_fn', 1)" - ), - {"r": repo_id, "f": removed_file_id}, - ) - conn.commit() assert _count(conn, "reference_edges", f"file_id = {removed_file_id}") == 1 # Reads above autobegan a txn; clear it so index_repo gets a clean connection @@ -153,7 +169,7 @@ def test_mark_and_sweep_removes_deleted_file(conn: Connection) -> None: # Re-run without util.py and with a new head SHA -> util.py is swept. counts = _index_default(conn, name="acme/widgets", head_sha="sha_second", items=_items(MAIN)) - assert counts == IndexCounts(files=1, symbols=1, swept=1) + assert counts == IndexCounts(files=1, symbols=1, swept=1, edges=0) assert _count(conn, "files", "path = 'util.py'") == 0 assert _count(conn, "symbols", f"file_id = {removed_file_id}") == 0 # cascade @@ -176,7 +192,7 @@ def test_sweep_is_repo_scoped(conn: Connection) -> None: # Re-index A without util.py at a new SHA -> A's util.py swept, B untouched. counts = _index_default(conn, name="acme/a", head_sha="a_second", items=_items(MAIN)) - assert counts == IndexCounts(files=1, symbols=1, swept=1) + assert counts == IndexCounts(files=1, symbols=1, swept=1, edges=0) assert _count(conn, "files", "repo_id = (SELECT id FROM repos WHERE name = 'acme/b')") == ( b_files_before @@ -184,6 +200,126 @@ def test_sweep_is_repo_scoped(conn: Connection) -> None: assert _count(conn, "files", "commit = 'b_first'") == 1 # B's row unchanged +# --- Reference edges: writer, stale replacement, idempotency, zero-edge shed --- + + +@pytest.mark.integration +def test_indexing_writes_correct_reference_edge_rows(conn: Connection) -> None: + symbol = ExtractedSymbol("f", "function", 1, 3) + item = ( + "main.py", + "def f():\n helper()\n return 1\n", + [symbol], + [ExtractedEdge(kind="call", target="helper", line=2, enclosing=symbol)], + ) + counts = _index_default(conn, name="acme/widgets", head_sha="sha_first", items=_items(item)) + assert counts == IndexCounts(files=1, symbols=1, swept=0, edges=1) + + repo_id = conn.execute(text("SELECT id FROM repos WHERE name = 'acme/widgets'")).scalar_one() + file_id = conn.execute(text("SELECT id FROM files WHERE path = 'main.py'")).scalar_one() + row = conn.execute( + text( + "SELECT repo_id, file_id, edge_kind, target_name, line, " + "enclosing_name, enclosing_kind, enclosing_start_line, enclosing_end_line " + "FROM reference_edges WHERE file_id = :f" + ), + {"f": file_id}, + ).one() + assert row == (repo_id, file_id, "call", "helper", 2, "f", "function", 1, 3) + + +@pytest.mark.integration +def test_reindex_replaces_stale_edges_for_the_same_file(conn: Connection) -> None: + """Same file identity (content/content_sha unchanged) across two runs: edges are + deleted and reinserted, not accumulated -- proven by driving the two runs' + ``ex.edges`` directly rather than depending on the real extractor to disagree + with itself on unchanged content. + """ + symbol = ExtractedSymbol("f", "function", 1, 3) + content = "def f():\n target()\n return 1\n" + first_edge = ExtractedEdge(kind="call", target="old_target", line=2, enclosing=symbol) + _index_default( + conn, + name="acme/widgets", + head_sha="sha_first", + items=_items(("main.py", content, [symbol], [first_edge])), + ) + file_id = conn.execute(text("SELECT id FROM files WHERE path = 'main.py'")).scalar_one() + assert _count(conn, "reference_edges", f"file_id = {file_id}") == 1 + conn.rollback() + + second_edge = ExtractedEdge(kind="call", target="new_target", line=2, enclosing=symbol) + _index_default( + conn, + name="acme/widgets", + head_sha="sha_first", + items=_items(("main.py", content, [symbol], [second_edge])), + ) + + same_file_id = conn.execute(text("SELECT id FROM files WHERE path = 'main.py'")).scalar_one() + assert same_file_id == file_id + targets = ( + conn.execute( + text("SELECT target_name FROM reference_edges WHERE file_id = :f"), {"f": file_id} + ) + .scalars() + .all() + ) + assert targets == ["new_target"] + + +@pytest.mark.integration +def test_reindex_with_identical_items_does_not_duplicate_edges(conn: Connection) -> None: + symbol = ExtractedSymbol("f", "function", 1, 3) + item = ( + "main.py", + "def f():\n helper()\n return 1\n", + [symbol], + [ExtractedEdge(kind="call", target="helper", line=2, enclosing=symbol)], + ) + _index_default(conn, name="acme/widgets", head_sha="sha_first", items=_items(item)) + conn.rollback() + _index_default(conn, name="acme/widgets", head_sha="sha_first", items=_items(item)) + + file_id = conn.execute(text("SELECT id FROM files WHERE path = 'main.py'")).scalar_one() + assert _count(conn, "reference_edges", f"file_id = {file_id}") == 1 + + +@pytest.mark.integration +def test_reindex_to_zero_edges_sheds_all_rows(conn: Connection) -> None: + """The unconditional-delete guard: same file identity, edges vanish on re-index. + + Content (and thus ``content_sha``) is held IDENTICAL across both runs so the + upsert resolves to the SAME ``file_id`` -- isolating the write-side guard + (``ex.edges`` empty must still run the delete) from the unrelated + delete-and-reinsert-under-a-new-file-id path already covered above. + """ + symbol = ExtractedSymbol("f", "function", 1, 3) + content = "def f():\n helper()\n return 1\n" + edge = ExtractedEdge(kind="call", target="helper", line=2, enclosing=symbol) + _index_default( + conn, + name="acme/widgets", + head_sha="sha_first", + items=_items(("main.py", content, [symbol], [edge])), + ) + file_id = conn.execute(text("SELECT id FROM files WHERE path = 'main.py'")).scalar_one() + assert _count(conn, "reference_edges", f"file_id = {file_id}") == 1 + conn.rollback() + + # Same content (same file_id) but this run's extraction yields zero edges. + counts = _index_default( + conn, + name="acme/widgets", + head_sha="sha_second", + items=_items(("main.py", content, [symbol], [])), + ) + assert counts == IndexCounts(files=1, symbols=1, swept=0, edges=0) + same_file_id = conn.execute(text("SELECT id FROM files WHERE path = 'main.py'")).scalar_one() + assert same_file_id == file_id + assert _count(conn, "reference_edges", f"file_id = {file_id}") == 0 + + @pytest.mark.integration def test_stamp_writes_semantics_version(conn: Connection) -> None: _index_default(conn, name="acme/widgets", head_sha="sha_first", items=_items(MAIN, UTIL)) @@ -272,10 +408,13 @@ def test_midrun_failure_rolls_back_entirely(conn: Connection) -> None: symbols_before = _count(conn, "symbols") conn.rollback() # clear the read txn before the next index_repo (see note above) - def poison() -> Iterator[tuple[ParsedFile, list[ExtractedSymbol]]]: + def poison() -> Iterator[tuple[ParsedFile, FileExtraction]]: # First item is a NEW file that would be inserted; then blow up mid-stream # so the exception propagates out of index_repo's conn.begin(). - yield _pf("new.py", "def h():\n return 3\n"), [ExtractedSymbol("h", "function", 1, 2)] + yield ( + _pf("new.py", "def h():\n return 3\n"), + FileExtraction(symbols=[ExtractedSymbol("h", "function", 1, 2)], edges=[]), + ) raise RuntimeError("poison item") with pytest.raises(RuntimeError, match="poison item"): @@ -435,7 +574,7 @@ def test_per_branch_cas_resume_is_independent_per_branch(conn: Connection) -> No head_sha="sha_a2", items=_items(MAIN), ) - assert counts == IndexCounts(files=1, symbols=1, swept=0) + assert counts == IndexCounts(files=1, symbols=1, swept=0, edges=0) stamps = dict(conn.execute(text("SELECT branch, last_indexed_commit FROM repo_branches")).all()) assert stamps == {"a": "sha_a2", "b": "sha_b"} @@ -479,7 +618,7 @@ def test_empty_seen_set_skips_sweep_and_preserves_membership(conn: Connection) - counts = index_repo( conn, name="acme/widgets", branch="a", is_default=True, head_sha="sha_a2", items=[] ) - assert counts == IndexCounts(files=0, symbols=0, swept=0) + assert counts == IndexCounts(files=0, symbols=0, swept=0, edges=0) # main.py's membership in 'a' is untouched -- the sweep was skipped, not run. assert _count(conn, "files", "path = 'main.py'") == 1 diff --git a/tests/integration/test_store_chunk_writer.py b/tests/integration/test_store_chunk_writer.py index e32c45a..0dd67f5 100644 --- a/tests/integration/test_store_chunk_writer.py +++ b/tests/integration/test_store_chunk_writer.py @@ -25,7 +25,7 @@ from app.db.client import create_db_engine from app.db.models import Base from indexer.chunk_store import write_chunks -from indexer.languages import ExtractedSymbol, IndexCounts, ParsedFile +from indexer.languages import ExtractedSymbol, FileExtraction, IndexCounts, ParsedFile from indexer.store import index_repo SCHEMA = "test_store_chunk_writer" @@ -94,13 +94,22 @@ def _count(conn: Connection, table: str, where: str = "") -> int: UTIL = ("util.py", "def g():\n return 2\n", [ExtractedSymbol("g", "function", 1, 2)]) +def _items( + *specs: tuple[str, str, list[ExtractedSymbol]], +) -> list[tuple[ParsedFile, FileExtraction]]: + return [ + (_pf(path, content), FileExtraction(symbols=syms, edges=[])) + for path, content, syms in specs + ] + + @pytest.mark.integration def test_chunk_writer_none_is_byte_identical_to_the_core_path(conn: Connection) -> None: - items = [(_pf(path, content), syms) for path, content, syms in (MAIN, UTIL)] + items = _items(MAIN, UTIL) counts = index_repo( conn, name="acme/widgets", branch="main", is_default=True, head_sha="sha_first", items=items ) - assert counts == IndexCounts(files=2, symbols=2, swept=0) + assert counts == IndexCounts(files=2, symbols=2, swept=0, edges=0) assert _count(conn, "files") == 2 assert _count(conn, "symbols") == 2 assert _count(conn, "chunks") == 0 # no chunk_writer -> chunks untouched @@ -108,7 +117,7 @@ def test_chunk_writer_none_is_byte_identical_to_the_core_path(conn: Connection) @pytest.mark.integration def test_chunk_writer_writes_inside_the_transaction(conn: Connection) -> None: - items = [(_pf(path, content), syms) for path, content, syms in (MAIN, UTIL)] + items = _items(MAIN, UTIL) counts = index_repo( conn, name="acme/widgets", @@ -118,7 +127,7 @@ def test_chunk_writer_writes_inside_the_transaction(conn: Connection) -> None: items=items, chunk_writer=_stub_chunk_writer, ) - assert counts == IndexCounts(files=2, symbols=2, swept=0) + assert counts == IndexCounts(files=2, symbols=2, swept=0, edges=0) assert _count(conn, "chunks") == 2 main_file_id = conn.execute(text("SELECT id FROM files WHERE path = 'main.py'")).scalar_one() @@ -127,7 +136,7 @@ def test_chunk_writer_writes_inside_the_transaction(conn: Connection) -> None: @pytest.mark.integration def test_reindex_is_idempotent_for_chunks(conn: Connection) -> None: - items = [(_pf(path, content), syms) for path, content, syms in (MAIN, UTIL)] + items = _items(MAIN, UTIL) index_repo( conn, name="acme/widgets", @@ -147,13 +156,13 @@ def test_reindex_is_idempotent_for_chunks(conn: Connection) -> None: items=items, chunk_writer=_stub_chunk_writer, ) - assert counts == IndexCounts(files=2, symbols=2, swept=0) + assert counts == IndexCounts(files=2, symbols=2, swept=0, edges=0) assert _count(conn, "chunks") == 2 # delete-and-reinsert, not duplicated @pytest.mark.integration def test_chunks_cascade_delete_when_file_is_swept(conn: Connection) -> None: - items = [(_pf(path, content), syms) for path, content, syms in (MAIN, UTIL)] + items = _items(MAIN, UTIL) index_repo( conn, name="acme/widgets", @@ -168,7 +177,7 @@ def test_chunks_cascade_delete_when_file_is_swept(conn: Connection) -> None: conn.rollback() # Re-index without util.py at a new SHA -> util.py (and its chunks) swept. - main_only = [(_pf(*MAIN[:2]), MAIN[2])] + main_only = _items(MAIN) counts = index_repo( conn, name="acme/widgets", @@ -178,7 +187,7 @@ def test_chunks_cascade_delete_when_file_is_swept(conn: Connection) -> None: items=main_only, chunk_writer=_stub_chunk_writer, ) - assert counts == IndexCounts(files=1, symbols=1, swept=1) + assert counts == IndexCounts(files=1, symbols=1, swept=1, edges=0) assert _count(conn, "files", "path = 'util.py'") == 0 assert _count(conn, "chunks", f"file_id = {util_file_id}") == 0 # cascade assert _count(conn, "chunks") == 1 diff --git a/tests/unit/AGENTS.md b/tests/unit/AGENTS.md index e627ba0..c12bc22 100644 --- a/tests/unit/AGENTS.md +++ b/tests/unit/AGENTS.md @@ -15,13 +15,14 @@ Hermetic unit tests: no network, no database, no Databricks SDK instantiation. E | `test_chunking.py` | `indexer.parse.iter_chunks` chunking behavior. | | `test_ci_branch.py` | `scripts/ci_branch.py` lifecycle with the SDK fully faked; pins that teardown NEVER raises and every create carries a TTL (leak protection for cancelled CI runs). | | `test_db_client.py` | Engine factory local (`PGHOST`) mode builds without instantiating the SDK; ORM models expose exactly the durable-core columns, constraints, and GIN indexes. | +| `test_edges.py` | `indexer.symbols.extract_file`'s reference-edge extraction (#84, Python): call-target resolution (rightmost identifier), import-target resolution (dotted paths, aliases, relative imports, wildcards), enclosing-symbol attribution, non-Python languages yield no edges, `extract_symbols` wrapper equivalence, determinism. | | `test_embed.py` | `app.embed`: batching, retry, dim-mismatch, lazy SDK import — every test injects a fake `client`, `databricks.sdk` is never imported. | | `test_fetch.py` | `indexer.fetch` via `httpx.MockTransport` + in-memory tarballs. | | `test_grants.py` | Least-privilege grant builders: presence AND absence of privileges per role; hostile identifiers rejected before SQL is produced. | | `test_grep.py` | Grep line extraction + matcher building (`extract_line_matches`, `_build_matchers`); byte-offset invariant `line_text.encode("utf-8")[s:e] == matched`. | | `test_job.py` | `indexer.job`: `read_github_token` + orchestration with every I/O boundary faked (fake `WorkspaceClient`, injected `config_loader`, etc.). | | `test_job_redaction.py` | GitHub-token redaction proof + source-level tripwire (two independent guards). | -| `test_languages.py` | Language/symbol source-of-truth maps: `SYMBOL_KINDS` ⊆ `EXT_TO_LANG` values, no orphans. | +| `test_languages.py` | Language/symbol source-of-truth maps: `SYMBOL_KINDS` ⊆ `EXT_TO_LANG` values, no orphans; `EDGE_NODE_KINDS` ⊆ `EXT_TO_LANG` values, no orphans; every `EDGE_NODE_KINDS` kind is within `reference_edges`' DB CHECK set. | | `test_main.py` | MCP server payload builders (`_search_code_payload` / `_list_repos_payload` / `_get_file_payload`), error mapping, and the `observability`-marked logging choke-point tests; fake engine/connection + fake `GrepResult`. | | `test_migration_source.py` | Static source assertions on the linear migrations: fixed revision ids, `down_revision` chain, `pg_trgm` invariants. | | `test_migration_source_semantic.py` | Static source assertions on the semantic `0004` revision (its DDL only runs on a Lakebase branch, so source reads are the unit-tier guard). | diff --git a/tests/unit/test_edges.py b/tests/unit/test_edges.py new file mode 100644 index 0000000..ff769e0 --- /dev/null +++ b/tests/unit/test_edges.py @@ -0,0 +1,183 @@ +"""Unit tests for indexer.symbols.extract_file's reference-edge extraction (Python, #84). + +Mirrors test_symbols.py's style: pure tree-sitter parsing, no DB. Covers call-target +resolution (D4), import-target resolution (D5), and enclosing attribution (D6). +""" + +from __future__ import annotations + +import pytest + +from indexer.languages import ExtractedEdge, ParsedFile +from indexer.symbols import extract_file, extract_symbols + + +def _pf(content: str, lang: str | None = "python") -> ParsedFile: + return ParsedFile(path="x.py", lang=lang, size=len(content), content=content) + + +def _edges(content: str) -> list[ExtractedEdge]: + return extract_file(_pf(content)).edges + + +@pytest.mark.unit +def test_bare_call_at_top_level() -> None: + edges = _edges("f(x)\n") + assert edges == [ExtractedEdge(kind="call", target="f", line=1, enclosing=None)] + + +@pytest.mark.unit +def test_nested_calls_two_edges_correct_lines() -> None: + edges = _edges("f(g(x))\n") + assert [(e.target, e.line) for e in edges] == [("f", 1), ("g", 1)] + + +@pytest.mark.unit +def test_method_and_bare_calls_both_use_rightmost_identifier() -> None: + edges = _edges("self.helper()\nhelper()\n") + assert [e.target for e in edges] == ["helper", "helper"] + + +@pytest.mark.unit +def test_dotted_callee_uses_rightmost_identifier() -> None: + edges = _edges("a.b.f()\n") + assert [e.target for e in edges] == ["f"] + + +@pytest.mark.unit +def test_enclosing_attribution_function_method_class_and_module() -> None: + content = ( + "def top():\n" + " call_in_fn()\n" + "\n" + "class C:\n" + " def m(self):\n" + " call_in_method()\n" + " call_in_class_body()\n" + "\n" + "call_at_module_scope()\n" + ) + fx = extract_file(_pf(content)) + by_target = {e.target: e for e in fx.edges} + + fn_edge = by_target["call_in_fn"] + assert fn_edge.enclosing is not None + assert fn_edge.enclosing.name == "top" + assert fn_edge.enclosing.kind == "function" + + method_edge = by_target["call_in_method"] + assert method_edge.enclosing is not None + assert method_edge.enclosing.name == "m" + assert method_edge.enclosing.kind == "function" + + class_edge = by_target["call_in_class_body"] + assert class_edge.enclosing is not None + assert class_edge.enclosing.name == "C" + assert class_edge.enclosing.kind == "class" + + module_edge = by_target["call_at_module_scope"] + assert module_edge.enclosing is None + + by_name = {s.name: s for s in fx.symbols} + assert method_edge.enclosing.start_line == by_name["m"].start_line + assert method_edge.enclosing.end_line == by_name["m"].end_line + assert class_edge.enclosing.start_line == by_name["C"].start_line + assert class_edge.enclosing.end_line == by_name["C"].end_line + + +@pytest.mark.unit +def test_decorator_with_args_emits_edge_bare_decorator_does_not() -> None: + content = "@deco(x)\ndef foo(): pass\n@bare\ndef bar(): pass\n" + edges = _edges(content) + assert [e.target for e in edges] == ["deco"] + + +@pytest.mark.unit +def test_non_identifier_callees_are_skipped() -> None: + edges = _edges("xs[0]()\nf()()\n") + # xs[0]() has no rightmost identifier -> skipped. + # f()()'s outer call target is itself a `call` node -> skipped; the inner f() is counted once. + assert [e.target for e in edges] == ["f"] + + +@pytest.mark.unit +def test_import_plain_dotted_path() -> None: + edges = _edges("import a.b.c\n") + assert edges == [ExtractedEdge(kind="import", target="a.b.c", line=1, enclosing=None)] + + +@pytest.mark.unit +def test_import_alias_is_insensitive_to_binding_name() -> None: + edges = _edges("import a.b.c as d\n") + assert [e.target for e in edges] == ["a.b.c"] + + +@pytest.mark.unit +def test_import_multiple_names_two_edges() -> None: + edges = _edges("import a, b\n") + assert [e.target for e in edges] == ["a", "b"] + + +@pytest.mark.unit +def test_from_import_names_and_alias() -> None: + edges = _edges("from a.b import c, d as e\n") + assert [e.target for e in edges] == ["a.b.c", "a.b.d"] + + +@pytest.mark.unit +def test_relative_import_single_dot() -> None: + edges = _edges("from . import x\n") + assert [e.target for e in edges] == [".x"] + + +@pytest.mark.unit +def test_relative_import_double_dot_with_module() -> None: + edges = _edges("from ..p import q\n") + assert [e.target for e in edges] == ["..p.q"] + + +@pytest.mark.unit +def test_wildcard_import_targets_the_module() -> None: + edges = _edges("from a.b import *\n") + assert edges == [ExtractedEdge(kind="import", target="a.b", line=1, enclosing=None)] + + +@pytest.mark.unit +def test_multiline_parenthesized_from_import_per_name_lines() -> None: + content = "from a.b import (\n c,\n d,\n)\n" + edges = _edges(content) + assert [(e.target, e.line) for e in edges] == [("a.b.c", 2), ("a.b.d", 3)] + + +@pytest.mark.unit +def test_function_local_import_attributes_to_enclosing_function() -> None: + content = "def outer():\n import os\n" + fx = extract_file(_pf(content)) + assert len(fx.edges) == 1 + edge = fx.edges[0] + assert edge.target == "os" + assert edge.enclosing is not None + assert edge.enclosing.name == "outer" + + +@pytest.mark.unit +def test_non_python_languages_and_none_lang_yield_no_edges() -> None: + js_content = "f(x);\nimport { a } from 'b';\n" + assert extract_file(_pf(js_content, lang="javascript")).edges == [] + assert extract_file(_pf("f(x)\n", lang=None)).edges == [] + + +@pytest.mark.unit +def test_extract_symbols_is_a_thin_wrapper_over_extract_file() -> None: + content = "class C:\n def m(self):\n helper()\n" + pf = _pf(content) + assert extract_symbols(pf) == extract_file(pf).symbols + + +@pytest.mark.unit +def test_extraction_is_deterministic() -> None: + content = "import a.b\nclass C:\n def m(self):\n helper()\n other.call()\n" + pf = _pf(content) + first = extract_file(pf).edges + for _ in range(5): + assert extract_file(pf).edges == first diff --git a/tests/unit/test_job.py b/tests/unit/test_job.py index 28f326a..85c1b30 100644 --- a/tests/unit/test_job.py +++ b/tests/unit/test_job.py @@ -309,8 +309,9 @@ def __call__( self.calls.append(name) self.chunk_writer = chunk_writer files = len(materialized) - symbols = sum(len(syms) for _pf, syms in materialized) - counts = IndexCounts(files=files, symbols=symbols, swept=0) + symbols = sum(len(ex.symbols) for _pf, ex in materialized) + edges = sum(len(ex.edges) for _pf, ex in materialized) + counts = IndexCounts(files=files, symbols=symbols, swept=0, edges=edges) self.counts.append(counts) return counts @@ -428,7 +429,7 @@ def test_run_parses_files_and_symbols() -> None: assert code == 0 # main.py + README.md both stored; main.py yields one function symbol. assert idx.calls == ["acme/widgets"] - assert idx.counts == [IndexCounts(files=2, symbols=1, swept=0)] + assert idx.counts == [IndexCounts(files=2, symbols=1, swept=0, edges=0)] # --- import health (the circular-import regression guard) ------------------- @@ -666,7 +667,7 @@ def test_semantic_ceiling_exceeded_degrades_but_still_indexes_the_core() -> None assert idx.chunk_writer is None # ...with chunks skipped # Proves the core index got the real work, not an empty items generator: "not skipped" # and "correctly indexed" are different claims, and only the latter is the contract. - assert idx.counts[0] == IndexCounts(files=2, symbols=1, swept=0) + assert idx.counts[0] == IndexCounts(files=2, symbols=1, swept=0, edges=0) @pytest.mark.unit @@ -708,7 +709,7 @@ def test_semantic_cap_override_exceeded_still_degrades_to_core_index() -> None: assert code == 0 # a semantic-only breach never fails the repo assert idx.calls == ["acme/widgets"] assert idx.chunk_writer is None # the override (1), not the global cap (5), fired - assert idx.counts[0] == IndexCounts(files=2, symbols=1, swept=0) + assert idx.counts[0] == IndexCounts(files=2, symbols=1, swept=0, edges=0) @pytest.mark.unit @@ -1173,7 +1174,7 @@ def _index(conn: Any, *, name: str, branch: str, **_: Any) -> IndexCounts: seen.append(branch) if branch == "main": raise StaleIndexError(f"repo_branches row for {name}@{branch} changed") - return IndexCounts(files=1, symbols=0, swept=0) + return IndexCounts(files=1, symbols=0, swept=0, edges=0) with caplog.at_level(logging.INFO, logger="indexer.job"): code = _run(_config(repos=["acme/widgets"], branches=["feature"]), _index, github=github) @@ -1195,7 +1196,7 @@ def _index(conn: Any, *, name: str, branch: str, **_: Any) -> IndexCounts: seen.append(branch) if branch == "main": raise RuntimeError("boom") - return IndexCounts(files=1, symbols=0, swept=0) + return IndexCounts(files=1, symbols=0, swept=0, edges=0) code = _run(_config(repos=["acme/widgets"], branches=["feature"]), _index, github=github) @@ -1343,7 +1344,7 @@ def __call__(self, conn: Any, *, name: str, items: Any, **_: Any) -> IndexCounts list(items) self.barrier.wait() self.calls.append(name) - return IndexCounts(files=0, symbols=0, swept=0) + return IndexCounts(files=0, symbols=0, swept=0, edges=0) @pytest.mark.unit @@ -1424,7 +1425,7 @@ def _mixed(conn: Any, *, name: str, items: Any, **_: Any) -> IndexCounts: list(items) if name == "acme/conflicted": raise StaleIndexError(f"repos row for {name} changed mid-transaction") - return IndexCounts(files=1, symbols=0, swept=0) + return IndexCounts(files=1, symbols=0, swept=0, edges=0) engine = _FakeEngine( stamps={("acme/skipped", "main"): ("sha_skipped", INDEX_SEMANTICS_VERSION)} @@ -1462,7 +1463,7 @@ def _slow(conn: Any, *, name: str, items: Any, **_: Any) -> IndexCounts: list(items) time.sleep(0.05) finished.append(time.monotonic()) - return IndexCounts(files=0, symbols=0, swept=0) + return IndexCounts(files=0, symbols=0, swept=0, edges=0) engine = _FakeEngine() monkeypatch.setattr(job, "create_db_engine", lambda **_kw: engine) @@ -1535,7 +1536,7 @@ def test_records_from_other_modules_inherit_the_repo_context( def _logs_elsewhere(conn: Any, *, name: str, items: Any, **_: Any) -> IndexCounts: list(items) logging.getLogger("indexer.store").warning("a record from another module") - return IndexCounts(files=0, symbols=0, swept=0) + return IndexCounts(files=0, symbols=0, swept=0, edges=0) log_filter = job.RepoLogFilter() caplog.handler.addFilter(log_filter) @@ -1611,12 +1612,12 @@ def __init__(self) -> None: def __call__(self, conn: Any, *, name: str, items: Any, **_: Any) -> IndexCounts: collected: list[tuple[str, str, str, str, int, int]] = [] - for pf, syms in items: - for sym in syms: + for pf, ex in items: + for sym in ex.symbols: assert isinstance(sym, ExtractedSymbol) collected.append((name, pf.path, sym.name, sym.kind, sym.start_line, sym.end_line)) self.rows.extend(collected) - return IndexCounts(files=0, symbols=len(collected), swept=0) + return IndexCounts(files=0, symbols=len(collected), swept=0, edges=0) @property def sorted_rows(self) -> list[tuple[str, str, str, str, int, int]]: @@ -1833,7 +1834,7 @@ def _ok_outcome( discovery_complete=discovery_complete, outcomes=[ BranchOutcome( - branch=b, status="indexed", counts=IndexCounts(files=1, symbols=0, swept=0) + branch=b, status="indexed", counts=IndexCounts(files=1, symbols=0, swept=0, edges=0) ) for b in branches ], @@ -2023,7 +2024,7 @@ def test_reconciliation_runs_after_fanout_drains() -> None: def _index_fn(conn: Any, *, name: str, items: Any, **_: Any) -> IndexCounts: list(items) order.append(f"index:{name}") - return IndexCounts(files=1, symbols=0, swept=0) + return IndexCounts(files=1, symbols=0, swept=0, edges=0) class _OrderedReconcile(_RecordingReconcile): def removed_fn(self, conn: Any, *, desired_repos: Any) -> list[str]: diff --git a/tests/unit/test_job_redaction.py b/tests/unit/test_job_redaction.py index 7f78f57..8a2dfda 100644 --- a/tests/unit/test_job_redaction.py +++ b/tests/unit/test_job_redaction.py @@ -129,7 +129,7 @@ def _index_fn( chunk_writer: Any = None, ) -> IndexCounts: files = len(list(items)) - return IndexCounts(files=files, symbols=0, swept=0) + return IndexCounts(files=files, symbols=0, swept=0, edges=0) # No-op reconcile fns: a clean run in these tests always passes diff --git a/tests/unit/test_languages.py b/tests/unit/test_languages.py index b876f16..1786c1a 100644 --- a/tests/unit/test_languages.py +++ b/tests/unit/test_languages.py @@ -3,9 +3,11 @@ from __future__ import annotations import pytest +from sqlalchemy import CheckConstraint from tree_sitter_language_pack import get_parser -from indexer.languages import EXT_TO_LANG, MAX_FILE_BYTES, SYMBOL_KINDS +from app.db.models import ReferenceEdge +from indexer.languages import EDGE_NODE_KINDS, EXT_TO_LANG, MAX_FILE_BYTES, SYMBOL_KINDS @pytest.mark.unit @@ -15,6 +17,35 @@ def test_symbol_kind_languages_have_no_orphans() -> None: assert set(SYMBOL_KINDS) <= ext_langs, "SYMBOL_KINDS has a language with no extension mapping" +@pytest.mark.unit +def test_edge_node_kind_languages_have_no_orphans() -> None: + """Every language in EDGE_NODE_KINDS must be a value in EXT_TO_LANG.""" + ext_langs = set(EXT_TO_LANG.values()) + assert set(EDGE_NODE_KINDS) <= ext_langs, ( + "EDGE_NODE_KINDS has a language with no extension mapping" + ) + + +@pytest.mark.unit +def test_edge_node_kinds_are_within_the_db_check_set() -> None: + """Every EDGE_NODE_KINDS value must satisfy reference_edges' edge_kind CHECK constraint. + + Cross-checked against the constraint's actual SQL text -- the + languages-map<->schema tripwire -- rather than a hardcoded duplicate set, so + a schema change that narrows/renames the allowed kinds is caught here too. + """ + check = next( + c + for c in ReferenceEdge.__table__.constraints + if isinstance(c, CheckConstraint) and c.name == "ck_reference_edges_edge_kind" + ) + sql = str(check.sqltext) + allowed = {kind.strip().strip("'") for kind in sql.split("IN")[1].strip(" ()").split(",")} + + mapped_kinds = {kind for kinds in EDGE_NODE_KINDS.values() for kind in kinds.values()} + assert mapped_kinds <= allowed, f"EDGE_NODE_KINDS has a kind outside the DB CHECK set: {sql!r}" + + @pytest.mark.unit @pytest.mark.parametrize("lang", sorted(set(EXT_TO_LANG.values()))) def test_get_parser_succeeds_for_each_language(lang: str) -> None: diff --git a/tests/unit/test_store_chunk_writer.py b/tests/unit/test_store_chunk_writer.py index 037a322..a4a788b 100644 --- a/tests/unit/test_store_chunk_writer.py +++ b/tests/unit/test_store_chunk_writer.py @@ -16,7 +16,7 @@ import pytest from sqlalchemy import Delete, Insert, Update -from indexer.languages import ExtractedSymbol, IndexCounts, ParsedFile +from indexer.languages import ExtractedSymbol, FileExtraction, IndexCounts, ParsedFile from indexer.store import StaleIndexError, index_repo @@ -73,6 +73,10 @@ def execute(self, stmt: Any, params: Any = None) -> _FakeResult: return _FakeResult() if isinstance(stmt, Delete) and table == "symbols": return _FakeResult() + if isinstance(stmt, Insert) and table == "reference_edges": + return _FakeResult() + if isinstance(stmt, Delete) and table == "reference_edges": + return _FakeResult() if isinstance(stmt, Update) and table == "repo_branches": return _FakeResult(rowcount=self._stamp_rowcount) raise AssertionError(f"unexpected statement against {table!r}: {stmt}") @@ -84,7 +88,12 @@ def _pf(path: str, content: str) -> ParsedFile: @pytest.mark.unit def test_chunk_writer_defaults_to_none_and_behavior_is_unchanged() -> None: - items = [(_pf("a.py", "x = 1\n"), [ExtractedSymbol("x", "variable", 1, 1)])] + items = [ + ( + _pf("a.py", "x = 1\n"), + FileExtraction(symbols=[ExtractedSymbol("x", "variable", 1, 1)], edges=[]), + ) + ] counts = index_repo( _FakeConn(), name="acme/widgets", @@ -93,7 +102,7 @@ def test_chunk_writer_defaults_to_none_and_behavior_is_unchanged() -> None: head_sha="sha1", items=items, ) - assert counts == IndexCounts(files=1, symbols=1, swept=0) + assert counts == IndexCounts(files=1, symbols=1, swept=0, edges=0) @pytest.mark.unit @@ -104,8 +113,8 @@ def chunk_writer(conn: Any, repo_id: int, file_id: int, pf: ParsedFile) -> None: calls.append((repo_id, file_id, pf.path)) items = [ - (_pf("a.py", "x = 1\n"), []), - (_pf("b.py", "y = 2\n"), []), + (_pf("a.py", "x = 1\n"), FileExtraction(symbols=[], edges=[])), + (_pf("b.py", "y = 2\n"), FileExtraction(symbols=[], edges=[])), ] index_repo( _FakeConn(), @@ -122,7 +131,7 @@ def chunk_writer(conn: Any, repo_id: int, file_id: int, pf: ParsedFile) -> None: @pytest.mark.unit def test_no_chunk_writer_means_no_extra_calls() -> None: # A None chunk_writer must never itself be invoked (it isn't callable). - items = [(_pf("a.py", "x = 1\n"), [])] + items = [(_pf("a.py", "x = 1\n"), FileExtraction(symbols=[], edges=[]))] # No AttributeError/TypeError from trying to call None -> proves the `if # chunk_writer is not None` guard is doing its job. index_repo( @@ -140,7 +149,7 @@ def test_no_chunk_writer_means_no_extra_calls() -> None: def test_stamp_matching_no_row_raises_stale_index_error() -> None: # The CAS UPDATE matching zero rows means the repo_branches row moved out # from under the statement-2 baseline; index_repo must abort rather than stamp. - items = [(_pf("a.py", "x = 1\n"), [])] + items = [(_pf("a.py", "x = 1\n"), FileExtraction(symbols=[], edges=[]))] with pytest.raises(StaleIndexError, match="acme/widgets"): index_repo( _FakeConn(stamp_rowcount=0),