h-dv/code-index

Watch

Fork 0

Retire crates/plugins: 24k lines of extractors no release contains are still maintained twice; default build differs from release #515

New issue

Open

opened 2026-11-09 08:03:45 +01:00 by buildagent · 1 comment

buildagent commented

2026-11-09 08:03:45 +01:00

Member

Consolidated scope (triage 2026-11-09)

Priority: Medium: about 25k lines maintained twice; the default build is not the product.

This issue is now the canonical tracker for:

Epic closed; this is its last step (oracle as data, then delete). Port the tests that still grade only the frozen builtins first (#125's destructuring contract, #270's receiver_file_anchor) (from #309, closed and merged here; its full report stays readable there)
share the JS/TS walker behind a dialect flag (180-line diff); move common.rs/pos.rs and method_or_function into guest-kit (from #516, closed and merged here; its full report stays readable there)

Original report

Summary

crates/plugins holds about 24k source lines of language extractors (rust 3,864, typescript 3,787, python 3,731, csharp 3,671, ruby 3,282 and php 2,160 lines), plus about 5k lines of tests. No release build contains any of it. Every release cargo build passes --no-default-features (.forgejo/workflows/release.yml:711, 718, 799, 1250, 1257), and the running daemon reports "NO builtin for csharp, javascript, php, python, ruby, rust, typescript". Even so, the code is maintained as a second, drifting implementation:

The guest walkers in crates/guest/*/src started as copies and now differ from the builtins by 1,217–2,608 diff lines per language. crates/plugins/src has taken 7 commits since the 2026-10-08 freeze (git log --since=2026-10-08 -- crates/plugins/src), so fixes are written twice.
Keeping the frozen copy honest takes its own machinery: crates/indexer/tests/package_parity/ (deltas.rs 1,369 lines, mod.rs 1,823), first-party-tests/.../walker_parity.rs (1,115), crates/plugins/tests/builtin_frozen.rs, and a parity-deltas.toml per package.
Production code carries the builtin-vs-package ownership logic that release never reaches. search_text "builtin" finds over 1,200 hits in 64 production files, for example indexer/src/languages.rs 181, packages.rs 120, mcp-server/src/eligibility.rs 93 and index.rs 92. all_plugins() (crates/plugins/src/lib.rs:33-49) has 18 non-test call sites and returns an empty Vec in release.
There are three build shapes: release has no builtins, a default cargo build has builtin-ruby (default = ["builtin-ruby"] in the cli, daemon, indexer, mcp-server and plugins Cargo.toml), and tests have all of them. No test exercises the shape that ships.
crates/guest/languages/src/lib.rs:22, 40 pulls ../../../plugins/src/common.rs and pos.rs in with #[path]. crates/guest/csharp/src/common.rs and pos.rs are byte-identical copies (same git blobs 79177592… and a7de63d2…), even though crates/guest-kit exists for exactly this sharing.

Proposal

Set default = [] in all five manifests now, so that dev, CI and release build the same product. This is a one-line change in each.
Freeze the oracle as data, not code: record the builtin extractor outputs for the fixture and corpus inputs, as #49 already proposes, and grade packages against those recordings.
Delete crates/plugins/src/{rust,python,typescript,csharp,php,ruby}.rs, the builtin-* features, all_plugins/builtin_compiled and the in-process parse paths (embedded.rs extract_builtin, index.rs parse_with_plugin_within). Collapse the ownership logic to "package or not indexed".
Move common.rs and pos.rs into guest-kit. No #[path] may point into a host crate, which is the rule crates/guest/vue/Cargo.toml:7 already states.

Acceptance and risk

corpus_ratchet, precision_gate and the parity suites run against the recorded oracle and stay green with an unchanged baseline.json. Removing the code removes the oracle these suites compare against, so step 2 has to land first.

Related: #309 (epic), #49, #318. This issue makes the double-maintenance cost and the always-empty release path concrete. Found by automated bloat and architecture reviews (2026-11-09), with release.yml, the Cargo features and the blob hashes verified by hand.

Filed from a five-agent review of master @ e3ec6ead (bloat, robustness, agent usefulness, docs, architecture). Every finding was spot-checked in the main session before filing.

## Consolidated scope (triage 2026-11-09) **Priority: Medium**: about 25k lines maintained twice; the default build is not the product. This issue is now the canonical tracker for: - [ ] Epic closed; this is its last step (oracle as data, then delete). Port the tests that still grade only the frozen builtins first (#125's destructuring contract, #270's `receiver_file_anchor`) (from #309, closed and merged here; its full report stays readable there) - [ ] share the JS/TS walker behind a dialect flag (180-line diff); move `common.rs`/`pos.rs` and `method_or_function` into guest-kit (from #516, closed and merged here; its full report stays readable there) --- ## Original report ## Summary `crates/plugins` holds about 24k source lines of language extractors (rust 3,864, typescript 3,787, python 3,731, csharp 3,671, ruby 3,282 and php 2,160 lines), plus about 5k lines of tests. **No release build contains any of it.** Every release `cargo build` passes `--no-default-features` (`.forgejo/workflows/release.yml:711, 718, 799, 1250, 1257`), and the running daemon reports "NO builtin for csharp, javascript, php, python, ruby, rust, typescript". Even so, the code is maintained as a second, drifting implementation: - The guest walkers in `crates/guest/*/src` started as copies and now differ from the builtins by 1,217–2,608 diff lines per language. `crates/plugins/src` has taken **7 commits since the 2026-10-08 freeze** (`git log --since=2026-10-08 -- crates/plugins/src`), so fixes are written twice. - Keeping the frozen copy honest takes its own machinery: `crates/indexer/tests/package_parity/` (deltas.rs 1,369 lines, mod.rs 1,823), `first-party-tests/.../walker_parity.rs` (1,115), `crates/plugins/tests/builtin_frozen.rs`, and a `parity-deltas.toml` per package. - Production code carries the builtin-vs-package ownership logic that release never reaches. `search_text "builtin"` finds over 1,200 hits in 64 production files, for example `indexer/src/languages.rs` 181, `packages.rs` 120, `mcp-server/src/eligibility.rs` 93 and `index.rs` 92. `all_plugins()` (`crates/plugins/src/lib.rs:33-49`) has 18 non-test call sites and returns an empty Vec in release. - There are three build shapes: release has no builtins, a default `cargo build` has `builtin-ruby` (`default = ["builtin-ruby"]` in the cli, daemon, indexer, mcp-server and plugins `Cargo.toml`), and tests have all of them. **No test exercises the shape that ships.** - `crates/guest/languages/src/lib.rs:22, 40` pulls `../../../plugins/src/common.rs` and `pos.rs` in with `#[path]`. `crates/guest/csharp/src/common.rs` and `pos.rs` are byte-identical copies (same git blobs `79177592…` and `a7de63d2…`), even though `crates/guest-kit` exists for exactly this sharing. ## Proposal 1. Set `default = []` in all five manifests now, so that dev, CI and release build the same product. This is a one-line change in each. 2. Freeze the oracle as **data**, not code: record the builtin extractor outputs for the fixture and corpus inputs, as #49 already proposes, and grade packages against those recordings. 3. Delete `crates/plugins/src/{rust,python,typescript,csharp,php,ruby}.rs`, the `builtin-*` features, `all_plugins`/`builtin_compiled` and the in-process parse paths (`embedded.rs` `extract_builtin`, `index.rs` `parse_with_plugin_within`). Collapse the ownership logic to "package or not indexed". 4. Move `common.rs` and `pos.rs` into `guest-kit`. No `#[path]` may point into a host crate, which is the rule `crates/guest/vue/Cargo.toml:7` already states. ## Acceptance and risk `corpus_ratchet`, `precision_gate` and the parity suites run against the recorded oracle and stay green with an unchanged `baseline.json`. Removing the code removes the oracle these suites compare against, so step 2 has to land first. Related: #309 (epic), #49, #318. This issue makes the double-maintenance cost and the always-empty release path concrete. Found by automated bloat and architecture reviews (2026-11-09), with release.yml, the Cargo features and the blob hashes verified by hand. --- _Filed from a five-agent review of master @ e3ec6ead (bloat, robustness, agent usefulness, docs, architecture). Every finding was spot-checked in the main session before filing._

buildagent added the

labels

2026-11-09 08:03:45 +01:00

buildagent referenced this issue

2026-11-09 08:35:47 +01:00

Corpus gates grade stale package pins (csharp 0.1.4 vs shipped 0.1.11, js 0.1.3 vs 0.1.6, …), hiding the C# csharp_partial bind withdrawal #534

buildagent referenced this issue

2026-11-09 09:20:55 +01:00

Resolved references can name unrelated methods, global names and function-local types #531

buildagent added

and removed

labels

2026-11-09 09:21:01 +01:00

~~buildagent referenced this issue 2026-11-09 09:21:02 +01:00~~

New language packages still need host knowledge to resolve imports and project layouts #518

buildagent referenced this issue

2026-11-09 09:21:04 +01:00

test oracle: frozen authoritative targets for builtin and external plugin languages #49

buildagent referenced this issue

2026-11-09 09:21:38 +01:00

epic: every builtin language becomes a first-party plugin package #309

buildagent referenced this issue

2026-11-09 09:21:39 +01:00

JS and TS packages carry two forks of one walker (180-line diff) — share it behind a dialect flag #516

buildagent referenced this issue

2026-11-09 09:21:44 +01:00

Receiver-binding phantom: a read of pkg.digest binds a same-named pub field in another crate, and localising the receiver does not move it #270

buildagent commented

2026-11-12 22:23:11 +01:00

Author

Member

Priority raised to High. #581 supplies a second reason for this work besides double maintenance: workspace tests compile the builtin parsers in and hide the cost of the shipped shape. The same startup steps take 0.08 s in the workspace test build and 8.5 s in the shipped build. Qualification must exercise the shipped package-only shape; retiring the duplicate producers removes this blind spot.

Priority raised to **High**. #581 supplies a second reason for this work besides double maintenance: workspace tests compile the builtin parsers in and hide the cost of the shipped shape. The same startup steps take 0.08 s in the workspace test build and 8.5 s in the shipped build. Qualification must exercise the shipped package-only shape; retiring the duplicate producers removes this blind spot.

buildagent added

and removed

labels

2026-11-12 22:23:29 +01:00

buildagent referenced this issue

2026-11-12 22:23:31 +01:00

The nightly runs have been red on 11 of 13 completed runs since 2026-11-06 and nothing reports it: overhead budgets, two intermittent startup failures, a step-guard cascade #582