Repository navigation
docs(spec): language packs and a two-plane topology - #593
Conversation
Design for #146 and for adding languages: cgis is a build plane (pipeline → languages → resolver → storage) and a read plane (storage → query → surfaces) over a shared core, and the observe-only quotient cannot be enforced because one triad census averages the two (0.644 against pipeline_stage, 0.527 orchestrator, 0.492 layered_dag at b0d67f8). - A language becomes one package behind a LanguagePack contract (parser, project reader, classify, call rules) and one registry line; Go is the pilot that proves it. - Drift: one quotient per plane plus an enforced forbidden-edge list, replacing the single project-level binding. - Migration in eight PRs; steps 1–5 move code under a graph-identity test over a checked-in corpus and leave every graph's node and edge sets unchanged. Refs #146 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request proposes a design specification for introducing language packs and a two-plane topology (build and read planes) over a shared core to resolve drift gate enforcement issues and simplify adding new languages. The review feedback recommends defining the interfaces for 'ProjectReader' and 'CallRule' to avoid ambiguity, and clarifying how the 'unless: same_pack' conditional for forbidden edges is evaluated.
…age, a fuller contract An independent design review (with measurements) and Gemini on #593: - Plane quotients cannot be the enforced gate: the read plane scores 0.676 against layered_dag (cli/api/guardian reach storage beside query), and the build plane against orchestrator reduces to "no leaf-to-leaf edge". The gate is now forbidden edges with shrinking allowlists, extending test_architecture.py; planes stay observe-only reports. - Go breaks file = namespace unit: the contract gains scope_of (file for Python/TS, package directory for Go) and is_test; same-file tiebreaks become same-scope. - The contract gains index_facts, classify with a context (source file, file metadata, facts), named resolution steps with type normalizers, a fallback with its own confidence and import reconciliation, and cross_file_metadata; ProjectReader/ProjectFacts and the same_pack matching are defined (Gemini). - resolver → storage is uplift.py (8 edges): uplift and refresh_dispatch move to a pipeline package. The registry is lazy (#506) and the one entry point for CLI, MCP server and scripts. - The identity net compares stored graphs with full node metadata, under two hash seeds. Refs #146 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
/gemini review |
|
/guardian review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary: Reviewed the language packs design spec for type safety, ontology compliance, and test coverage. Found critical type annotation gaps in Protocol definitions, ontology violations in pack isolation rules, and missing test coverage for migration steps and forbidden-edge enforcement.
🤖 mistral-medium-latest · 9,502 prompt + 3,641 completion = 13,143 tokens · skeptic 20/20
|
|
||
| The generic chain, with today's positions of language logic: | ||
|
|
||
| 1. `local_type`, with `type_normalizers` applied inside it, as |
There was a problem hiding this comment.
🟡 [Ontology] — Potential ontology violation: unresolved calls not using raw_call: prefix
The design does not explicitly address how unresolved calls (e.g., in Go) will use the raw_call: prefix as required by the ontology conventions.
Fix: Clarify in the Go pilot section or ResolutionRules contract that unresolved calls must use the raw_call: prefix per ontology conventions.
Verified by gemini-2.5-flash
There was a problem hiding this comment.
Added in 05e899f under the Go pilot's Ontology point. The Go extractor emits unresolved calls as raw_call:<name>, like the others, uses only the existing NodeType/EdgeType values, and must pass the existing ontology-compliance tests on the Go corpus.
|
|
||
| ## Migration | ||
|
|
||
| **Safety net, written first.** The input must stay fixed while the code |
There was a problem hiding this comment.
🟡 [Test Coverage] — Missing test coverage for graph identity under refactoring
The design describes a graph identity test but does not explicitly state that it will be added to CI or that it covers all migration steps (1–7).
Fix: Explicitly state that tests/unit/test_graph_identity.py will be added to CI and that it must pass for all migration steps (1–7).
Verified by gemini-2.5-flash
There was a problem hiding this comment.
Clarified in 05e899f. The identity test is an ordinary unit test, so CI runs it on every PR from step 0 on. The Python/TS snapshot never changes, step 7 included; Go gets a corpus and snapshot of its own, so the pilot cannot quietly move the existing languages.
|
|
||
| Each step is one PR with the full gate, the independent reviewer and the | ||
| identity report. Steps 1–5 are mergeable one at a time. Each step re-measures | ||
| the god-object and self-drift baselines. A tolerance may only go down, or be |
There was a problem hiding this comment.
🟡 [Test Coverage] — Missing test coverage for tolerance ratcheting
The design states that tolerances may only go down but does not explicitly state that tests will enforce this rule (e.g., in CI).
Fix: Add explicit test requirements to enforce that tolerances may only go down or be set for new domains with measurements.
Verified by gemini-2.5-flash
|
|
||
| **Acceptance is the diff:** only `languages/go/`, the registry entry, the | ||
| `tree-sitter-go` dependency and tests. If Go needs an edit anywhere else, the | ||
| contract is missing something. The contract gets fixed, and the change is |
There was a problem hiding this comment.
🟡 [Ontology] — Potential ontology violation: Go pilot may not handle all edge cases
The Go pilot's acceptance criteria do not explicitly state that it must handle all ontology edge cases (e.g., unresolved calls, FQN conventions).
Fix: Add explicit ontology compliance requirements for the Go pilot, including handling of unresolved calls, FQN conventions, and NodeType/EdgeType assignments.
Verified by gemini-2.5-flash
There was a problem hiding this comment.
Covered by the same addition in 05e899f: raw_call: for unresolved calls, the existing enums only, and the ontology-compliance tests run on the Go corpus. Go's FQN convention is spelled out in the contract: scope_of returns the package directory, and the pilot's acceptance is its diff staying inside languages/go/.
There was a problem hiding this comment.
Code Review
This pull request introduces a design specification for language packs and an enforced topology using forbidden edges. The review feedback highlights two critical areas where the proposed design conflicts with the goal of lazy-loading language packs to prevent eager loading of tree-sitter grammars: first, the eager instantiation of all extractors via the extractors(source_roots) mapping at startup, and second, the need to eagerly import all language pack modules to construct the union of cross_file_metadata for the pipeline.
| - `pack_for_path(path) -> LanguagePack | None`, by extension; | ||
| - `pack_for_node(node) -> LanguagePack | None`, by `Node.language`, never by | ||
| re-sniffing a suffix; | ||
| - `extractors(source_roots)`, the mapping `IngestionPipeline` takes. |
There was a problem hiding this comment.
Providing a pre-built mapping of all extractors via extractors(source_roots) to the IngestionPipeline at startup will require eagerly instantiating all extractors. This eagerly loads their respective tree-sitter grammars, which directly conflicts with the goal of lazy-loading packs only on first use to avoid undoing #506. Consider having the pipeline query the registry dynamically per file/extension (e.g., via pack_for_path or a lazy extractor lookup) rather than passing a complete mapping of all extractors at startup.
There was a problem hiding this comment.
Agreed, fixed in 05e899f. The pipeline no longer takes a ready mapping of every extractor. It calls registry.extractor_for(path, source_roots) per file, and a pack's extractor is built when its first file is met, so a Python-only repository never loads the TS or Go grammars.
| **Incremental ingest.** `cross_file_metadata` lists, per node type, the | ||
| metadata keys other files resolve against. The pipeline's | ||
| `_CROSS_FILE_METADATA` (pipeline.py:32) becomes the union over packs: | ||
| `import_map`, `reexports`, `star_imports`, `self_types` and `annotated_type` | ||
| for Python, and so on per pack. |
There was a problem hiding this comment.
If the pipeline's _CROSS_FILE_METADATA is constructed as a union of cross_file_metadata across all registered packs, the pipeline will need to eagerly import every language pack module at startup to read this attribute. This eager importing would defeat the lazy-loading design and eagerly load all tree-sitter grammars. To maintain lazy loading, consider declaring these metadata keys statically within the registry or a lightweight configuration module, so the pipeline can retrieve them without importing the full language pack modules.
There was a problem hiding this comment.
Agreed, fixed in 05e899f. cross_file_metadata now lives in each pack's lightweight registry manifest (name, module path, extensions, keys), and registry.cross_file_metadata() returns the union without importing any pack.
…h step 7; Go ontology conventions Gemini on #593: a ready mapping of every extractor, and a cross_file_metadata union read from pack modules, would both import every grammar and undo #506. The registry now holds lightweight per-pack manifests (name, module, extensions, cross_file_metadata) and builds an extractor on its pack's first file. Guardian on #593: the identity test runs in CI from step 0 and the Python/TS snapshot never changes, step 7 included (Go gets its own); the tolerance ratchet is the existing tolerances.lock test; Go emits raw_call: and passes the ontology-compliance tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Step 0 (one PR): canonical graph dump/diff script, a checked-in Python/TS identity corpus with a snapshot compared under two hash seeds, the forbidden-edge gate with exact allowlists in test_architecture.py, and plane quotients reported observe-only. Steps 1–7 each get a detailed plan when the previous one has merged. Refs #146 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
1 participant
Design spec, for review before an implementation plan is written. Refs #146.
docs/specs/2026-10-14-language-packs-design.md.What it proposes:
pipeline → languages → resolver → storage) and read (storage → query → surfaces).languages/<lang>/package behind aLanguagePackcontract (parser, project reader, classify, call rules), plus one registry line.languages/go/.Docs only.
🤖 Generated with Claude Code