Files
colbymchenry--codegraph/docs/design/value-reference-edges-playbook.md
wehub-resource-sync 5f98960d22
Deploy site to GitHub Pages / build (push) Failing after 1s
Deploy site to GitHub Pages / deploy (push) Has been skipped
chore: import upstream snapshot with attribution
2026-07-13 12:02:56 +08:00

545 lines
41 KiB
Markdown
Raw Permalink Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Playbook: extend value-reference edges to a new language
**Purpose.** This is the operational runbook for adding + validating value-reference-edge
coverage for one more language. Point a fresh session at this file and say **"Start on
language X"** — it has everything: how the feature works, where the code is, the exact
validation recipe (with scripts), the per-language checklist, and the traps already hit.
Design rationale + the validation matrix already done live in the companion doc:
[`value-reference-edges.md`](./value-reference-edges.md). This file is the *how-to*.
---
## 0. "Start on language X" — do this in order
1. Read §1 (how it works) and §2 (current state) so you know the mechanism and what's done.
2. Do the **per-language wiring check** (§5 step AC) — this is where languages differ and
where most of the real work/decisions are. Do NOT skip: a wrong declarator node type or a
class-scope-vs-file-scope mismatch makes the feature silently emit nothing (or wrong edges).
3. Run the **validation sweep** (§4) on small/medium/large **public OSS** repos for that
language. Hunt FPs. **Fix FP clusters; record singletons.** (See §3 for what a real FP
looks like vs an acceptable one.)
4. Add a **row to the matrix** in `value-reference-edges.md` and a **test case** in
`__tests__/value-reference-edges.test.ts`.
5. Commit on a branch, open a PR. (§6 has the git workflow + how the prior PRs were done.)
Scope rule (hard): **never eval on the maintainer's own repos** — clone a real public OSS
repo for the language. (Memory: `agent-eval-targets-public-oss-only`.)
---
## 1. How value-reference edges work
**What:** a `references` edge with `metadata: { valueRef: true }` from a *reader symbol* to
the **file-scope `const`/`var` it reads**, same-file only. It exists so impact analysis
catches "change this constant / config object / lookup table → affect its readers" — a class
of change calls/imports/inheritance edges never captured (a const's consumers used to look
like "nothing depends on this").
**Where it flows:** straight into `getImpactRadius``codegraph impact` and the impact trail
in `codegraph_explore` / `codegraph_node`. No agent-behaviour change required. **The win is
impact-radius correctness** (a const 90 symbols read going from "1 affected" to "90"), *not*
agent read-reduction (see §4.3).
**Code — all in `src/extraction/tree-sitter.ts`:**
| Symbol | Role |
|---|---|
| `VALUE_REF_LANGS` (static Set) | languages the feature runs for. Currently `typescript`, `javascript`, `tsx`, `go`, `python`, `rust`, `ruby`, `c`, `java`, `csharp`, `php`, `scala`, `kotlin`, `swift`, `dart`, `pascal`. **Add the new language here.** |
| `valueRefsEnabled` | `process.env.CODEGRAPH_VALUE_REFS !== '0'` — default ON, env opts out. |
| `MAX_VALUE_REF_NODES` (20_000) | per-scope traversal cap (and the shadow-scan cap). |
| `captureValueRefScope(kind, name, id, node)` | called from `createNode` on every node. Records **targets** (file-scope `const`/`var`) and **reader scopes** (`function`/`method`/`const`/`var`). |
| `flushValueRefs()` | called once at end of `extract()`. Prunes shadowed targets, then for each reader scope walks its subtree for identifiers matching a target name and emits the edges. |
**The two gates inside `captureValueRefScope`** (what you may need to adjust per language):
- **Target gate:** `kind ∈ {constant, variable}` **and** `name.length >= 3` **and**
`/[A-Z_]/.test(name)` (distinctive name — dodges single-letter / all-lowercase shadowing)
**and** the node's parent id starts with `file:`, `class:`, or `module:` (file/class/module scope).
- **Reader gate:** `kind ∈ {function, method, constant, variable}`.
**The emit loop in `flushValueRefs`:** same-file only (targets + scopes are per-file, reset
each flush); deduped per `(reader, target)`; skips `isGeneratedFile(path)`; **prunes shadowed
targets** (see §3).
---
## 2. Current state (what's shipped + validated)
- **Default ON** for TS/JS/tsx + Go + Python + Rust + Ruby + C + Java + C# (`CODEGRAPH_VALUE_REFS=0` disables). Shipped in **PR #895**
(flip-on + the shadow prune); Go added in a later PR (the shadow-prune declarator switch +
`VALUE_REF_LANGS`); C added later still (extractor change to emit the nodes + the bare-identifier
misparse guard); Java + C# after that (field→constant kind switch for the const subset).
- **Validated S/M/L** in **TS, JS, tsx, Go, Python, Rust, Ruby, C, Java, and C#** — see the matrix in the
design doc. All clean: node count identical on/off, precision guards held, impact win
reproduced. Go required extending the shadow prune (per-grammar declarators) — the worked
example of "step B is load-bearing." **C required the Ruby treatment** (the extractor didn't emit
C file-scope const/var nodes at all) **plus** a C-specific FP guard (a macro-prefixed-prototype
misparse mints a bare-identifier "variable" named after the return type — skip bare-`identifier`
declarators). It was the worked example of "the §2b coverage table's *easy-path* guess can be
wrong — always do §5 step C (confirm the nodes exist) before trusting it."
- **Java + C# were the cleanest class-scope ("Ruby treatment") languages.** The constants already
extract — but as `field` kind, which the gate rejects. The whole change was emitting the const
*subset* as `constant`: an `isConst` predicate on each extractor (Java `static final`; C# `const`
/ `static readonly`) + a kind switch in `extractField`. **No new shadow-prune wiring** (method
locals are `variable_declarator`, already in the switch) and **no FP guards** (UPPER_SNAKE /
PascalCase fit the distinctive-name gate). Instance `final`/`readonly` fields correctly stay
`field`. Validated S/M/L: gson/commons-lang/guava, automapper/newtonsoft/efcore — 0 leaks, node
parity, big impact wins (`INDEX_NOT_FOUND` 4→165, `_resourceManager` 22→1664).
- **PHP was the cleanest of all — one reader-scan line.** Constants already extract as `constant`
(top-level + class), so the only change was teaching the reader-scan that a PHP constant
*reference* is a `name` node (bare `X`, or the const half of `self::X` / `Foo::X`). **No extractor
change, no prune wiring** (a `$var` local can't shadow a bare constant — different namespace).
Validated S/M/L (guzzle/monolog/laravel), all clean, 0 class/const collisions. The honest caveat:
**lower yield** — PHP reads constants cross-file far more than same-file (laravel 2,956 files → 86
edges), and value-refs is same-file only; still correct, just a smaller contribution.
- **Scala — an `object` is the constant scope.** Scala has no `static`; a singleton `object`'s `val`s
are the shared-constant idiom (`object Config { val Timeout = 30 }`). Top-level `val` already
extracted as `constant`, but object/class vals both came out as `field`. The fix: in the Scala
`val_definition` handler, walk to the enclosing definition — `object_definition` (or top-level) →
`constant`/`variable`; `class`/`trait`/`enum``field` (per-instance, like Java instance `final`).
Added `val_definition`/`var_definition` to the shadow prune (method-local `val` shadows). Reader-scan
needed nothing (refs are `identifier`). Minor known limitation: Scala uses `val`/`def`
interchangeably for members, so a camelCase val can share a name with a method — same-file name
matching can't tell them apart (bounded, like Ruby's sibling-class; sweep showed flagged collisions
were mostly real object vals read by siblings). Validated S/M/L (upickle/cats/pekko).
- **C++ was attempted and reverted — DON'T retry without solving parse fidelity first.** tree-sitter-cpp
mis-parses real template/macro-heavy C++ (and `.h` files route to the C grammar): class members and
parameters leak to file scope as bogus constants/variables. Two guards (skip `ERROR`-ancestor and
`compound_statement`-ancestor declarations) removed ~83% of gross leaks, but the residual pervades
even well-structured library source (template-class member leaks, amalgamated mega-headers,
`.h`-as-C++). It did not reach the precision bar of the other languages. See the C++ section below.
- **Kotlin = C + Scala + PHP techniques combined (and clean).** Nothing extracted before (property name
nests `property_declaration → variable_declaration → simple_identifier` — the C problem). Fix:
handle `property_declaration` in the Kotlin `visitNode` hook — pull the nested name, walk to the
enclosing definition for the kind (`object`/`companion object`/top-level → `constant`/`variable`;
`class``field` — the Scala rule; skip locals under a `function_body`/`init`/lambda), add
`simple_identifier` to the reader-scan (the PHP-`name` move), and `property_declaration` to the
shadow prune. Clean parse fidelity (the one `fun interface` misparse is already handled), so no
C++-style tail. One of the cleanest yields — companion-object bit-masks/state consts are a heavy
same-file-read idiom. Validated S/M/L (okio/coroutines/ktor); only the bounded val/def-or-class and
sibling-companion name overlaps remain (shared with Scala/Ruby).
- **Swift reused Kotlin + two Swift-specific touches.** Top-level `let` + `static let` in a type are
the shared constants (`enum`/`struct` namespace them); instance `let` stays `field`. Nested name
(`property_declaration → <name> pattern → simple_identifier`); reader-scan already covered
(`simple_identifier`, from Kotlin). Two new things: **(1) the target gate was widened to `struct:`/
`enum:` parents** — Swift namespaces constants there (`enum Constants { static let X }`), and every
other language's targets are `file:`/`class:`/`module:`; **(2) computed properties are skipped** (a
`var x:Int{ … }` getter has no stored value — detect the `computed_property` child). Node creation
slots into the *existing* Swift `property_declaration` handler (property-wrapper/type deps), leaving
that untouched. Clean parse, no tail. Validated S/M/L (Alamofire/swift-argument-parser/swift-nio).
- **Dart — clean grammar separation, but a sibling-body reader-scan fix.** Dart's grammar already
splits the cases: **`static_final_declaration`** is *exactly* a top-level/`static` `const`/`final`
(the shared-constant idiom), while instance fields/`var` use `initialized_identifier` and locals use
`initialized_variable_definition` — so extracting `static_final_declaration``constant` (in a
`visitNode` hook) has **no instance/local leaks to guard**. Reader-scan free (Dart refs are
`identifier`). The catch was the **reader-scan**: Dart attaches a method/function `body` as a *next
sibling* of the signature node (the stored scope), not a child, so the scan saw only the signature
and **found nothing** until it was taught to pull in a `function_body` next-sibling (Dart-only among
the value-ref set). Shadow prune needed `static_final_declaration` + `initialized_identifier` +
`initialized_variable_definition` (a local `const X` shadowing a file `const X`). Validated S/M/L
(http/flame/flutter-packages). **Caveat:** generated Dart files inflate the sibling-class ambiguity
(a JNIGEN `_bindings.dart` with hundreds of `static final _class` collapses to the file-wide target).
The common codegen suffixes (`.g.dart`/`.freezed.dart`/`.pb.dart`) are already filtered by
`isGeneratedFile`; header-only-marked generators (JNIGEN) are not, so real source is clean but
generated FFI/JNI bindings are noisy.
- **Pascal — the genuine easy path + the Dart sibling-body fix again.** Unit/class `const` *already*
extracted as `constant` (`variableTypes: ['declConst', …]`), so it was add-to-`VALUE_REF_LANGS` +
the shadow prune (`declConst`/`declVar`; a local `const X` shadows a unit `const X`). The catch was
the *same* reader-scan bug as Dart: Pascal's proc body is a **`block` sibling** of the `declProc`
header (the reader scope), both under a `defProc` — so the same sibling-pull fix was extended to
`block`. Reader-scan node type already covered (refs are `identifier`). **Low yield** — Pascal reads
constants cross-unit more than same-file (horse: 4 edges). **Caveat:** Pascal is case-insensitive,
but the reader-scan matches exact text, so a differently-cased reference is missed (no FP, just a
miss); not worth normalizing.
- **Tests:** `__tests__/value-reference-edges.test.ts` — same-file readers edged; surfaced in
impact radius; shadowed const NOT edged (verified to fail without the guard); JSX-only read
edged (tsx); `CODEGRAPH_VALUE_REFS=0` emits nothing.
- **Memory:** `value-reference-edges-default-on` (the A/B finding + shadow guard rationale).
---
## 2b. Coverage vs the README (languages + frameworks)
Tracked against the README's **Supported Languages** table (24 rows) and **Framework-aware
Routes** list. Value-refs is **language-level**, so frameworks are *not* a separate axis (see
the bottom of this section).
**✅ Done — validated S/M/L (15 + 3 inherited):**
| Language | How |
|---|---|
| TypeScript, JavaScript, tsx | file-scope `const`/`var`; the original languages |
| Python | module-level `NAME =` |
| Go | package `const`/`var` |
| Rust | module + impl `const`/`static` |
| Ruby | class/module `CONST` (the class-scope extension) |
| C | file-scope `static const` scalars + pointer/array lookup tables + mutable globals. **Needed an extractor change** (nodes weren't emitted) + a bare-identifier misparse guard — NOT the easy path the table below first guessed |
| Java | class `static final` fields. Nodes existed as `field` kind; emitted the const subset as `constant` (`isConst` + `extractField` kind switch). No new prune wiring, no FP guards |
| C# | class `const` / `static readonly`. Identical to Java — same `field``constant` change |
| PHP | top-level `const` + class `const` (both already `constant` kind). **Only** change was the reader-scan: a PHP const *reference* is a `name` node. No extractor change, no prune wiring (a `$var` local can't shadow a bare constant). Lower yield — PHP reads consts cross-file more than same-file |
| Scala | top-level `val` (already `constant`) + **`object` val** (the singleton-constant idiom; re-kinded from `field` by walking to the enclosing `object_definition`). `class`/`trait`/`enum` vals stay `field`. `val_definition`/`var_definition` added to the shadow prune. Minor val/def name-collision limit |
| Kotlin | top-level / `object` / `companion object` `val` (re-kinded from nothing — properties weren't extracted at all). Handled in `visitNode`: nested name (`variable_declaration → simple_identifier`, the C move) + scope-walk for kind (Scala move) + `simple_identifier` in the reader-scan (PHP move) + prune. `class` instance vals stay `field`. Clean — one of the best yields (companion bit-masks) |
| Swift | top-level `let` + `static let` in `struct`/`enum`/`class`. Reused Kotlin (nested name + `simple_identifier` reader-scan). Two Swift touches: **gate widened to `struct:`/`enum:` parents** (Swift namespaces consts there), and **computed properties skipped**. `class`/instance stored props stay `field`. Slots into the existing Swift property-wrapper handler |
| Dart | top-level `const`/`final` + class `static const`/`static final` — all the **`static_final_declaration`** node, cleanly separated by the grammar from instance/`var`/local (so no leak guard). `visitNode``constant`. Needed a reader-scan fix: Dart's method **body is a next sibling** of the signature, so the scan pulls in a `function_body` sibling. Generated-FFI noise (JNIGEN `_bindings.dart`) is the one caveat |
| Pascal / Delphi | unit/class `const` (already extracted as `constant`). Add-to-`VALUE_REF_LANGS` + shadow prune (`declConst`/`declVar`) + the **same Dart sibling-body fix** (Pascal's proc body is a `block` sibling of the `declProc` header). Low yield (cross-unit reads); case-insensitive (exact-text scan misses re-cased refs) |
| **Svelte, Vue, Astro** | **inherited for free** — their extractors re-parse the `<script>`/frontmatter block as `typescript`/`javascript`, which are in `VALUE_REF_LANGS` (verified: a `.svelte` `const` edges its readers). No separate work; no separate matrix row needed. |
**🔜 Remaining — likely the easy path** (constants are file/module-scope, or top-level; do §5: add
to `VALUE_REF_LANGS`, verify the declarator node type + extractor kind, sweep). Classify each
*before* building — several are mixed file+class scope. **Caveat learned from C:** "easy path" here
means *scope* fits — it does NOT promise the extractor already emits the const nodes. C was in this
column but emitted *no* file-scope const/var nodes (its name nests in an `init_declarator` the
generic fallback can't read), so it needed the Ruby-style extractor change after all. **Always run
§5 step C (confirm `select kind,name from nodes …` actually shows the consts) before trusting this
column.**
| Language | Constant forms | Note |
|---|---|---|
| Lua / Luau | file/chunk `local X =` + globals; no `const` keyword | distinctive-name gate (needs `[A-Z_]`) catches fewer — Lua casing varies |
| R | file-scope `X <- …` / `X = …` | |
**🧱 Remaining — needs the Ruby treatment** (constants live almost entirely **inside a
class/type**; the class-scope *gate* exists now, but first confirm the extractor emits them as
`constant`/`variable` nodes — Ruby's weren't extracted at all, and class fields often come out as
`field`/`property` kind, which the gate rejects). **Java + C# (done) were this case**: their
constants extracted as `field` kind, and the fix was emitting the const subset (`static final` /
`const` / `static readonly`) as `constant` — the template for the rest of this bucket:
| Language | Constant forms |
|---|---|
| Objective-C | `static const` / `extern const` / `#define` (file-ish; macros unparsed; already "partial support") |
**⛔ Attempted & reverted — C++.** file-scope + class `static const`/`constexpr` (mixed). Machinery
built and correct on clean C++, but **tree-sitter-cpp parse fidelity is the blocker**: template/
macro-heavy real C++ leaks class members + parameters to file scope as bogus constants/variables, and
`.h` files route to the C grammar (mangling C++ classes). Two guards (skip `ERROR`-ancestor and
`compound_statement`-ancestor declarations) cut ~83% of gross leaks but the residual pervades even
well-structured library source. **Did not meet the precision bar; reverted.** Don't retry as a
"value-refs" task — it needs prior work on C++ parse handling (template-class member scoping,
`.h`-as-C++ detection, amalgamated-header exclusion).
**🚫 N/A:** Liquid (template language — no value constants to track).
**Frameworks — not a value-refs axis.** The README's framework list (Django, Flask, Express,
NestJS, Rails, Spring, Gin, Laravel, …) is a *separate* feature: **route-node extraction**.
Value-refs is framework-agnostic — it covers constants in any framework's code through the
underlying language support, with **nothing to do per framework**. The validation sweeps already
ran on framework repos (Rails → Ruby, Django → Python, gin → Go, express/eslint/webpack → JS,
jekyll/sinatra → Ruby), so framework code is exercised; there's no separate framework matrix.
---
## 3. Precision guards + what counts as a false positive
Guards run in `flushValueRefs`, in order:
1. **`isGeneratedFile(path)`** (`src/extraction/generated-detection.ts`) — skips
*suffix-recognised* generated files (`.pb.ts`, `.min.js`, …). **Path-only** — cannot catch
content-minified bundles.
2. **Shadow prune** — drop a target when its **declarator count exceeds its file-scope node
count** (so it's also bound in an inner/local scope). Rationale: a bundled/Emscripten `const
Module` re-declared as an inner `var Module`, a Go package const shadowed by a local `:=`, or
a Python module const shadowed by a local `=` resolves to the *inner* binding for nested
readers, so a file-scope edge is wrong. Inner re-bindings aren't graph nodes, so declarators
are counted at the **syntax-tree** level. *This is the per-language-sensitive guard:* the
declarator node types differ per grammar (§5 step B), and comparing against file-scope node
count (not a flat `>1`) is what keeps **conditional module defs** (`try: X=…; except: X=…`).
3. **Distinctive-name + same-file** (the target gate).
**What a real FP looks like** (fix it): a reader edged to a file-scope const it does **not**
actually read — almost always **intra-file shadowing** (the name is re-bound in an inner
scope) concentrated in **bundled/minified/generated** files. On excalidraw this was 23 edges
in one Emscripten blob.
**What is NOT an FP** (leave it):
- **CommonJS `var x = require('…')` bindings** (JS) — correct same-file reads; changing the
binding *does* affect its readers; dedups against `calls` edges in impact. Not noise.
- **Module-level mutable `var` state** read by many same-file functions — the intended case.
- A higher edge share in a language (JS ~45% vs TS ~0.71.6%) is fine if precision holds.
**Known limitations (intentional, documented):** parameter-only shadowing is *not* guarded
(the prune counts declarators, not params — guarding it would over-prune legit consts whose
name coincides with a param); same-file only (no cross-file consumers); reactive/computed
reads with no static identifier aren't covered.
---
## 4. Validation recipe
### 4.1 Deterministic probe (the core — finds FPs)
Index the same repo twice (on vs `CODEGRAPH_VALUE_REFS=0`); node count **must be identical**
(edges-only feature). Build first: `npm run build`. Save this as `probe.sh`:
```bash
#!/usr/bin/env bash
set -uo pipefail
SRC="$1"; NAME="$2"; WORK="${WORK:-/tmp/cg-vr}"
CG="$(pwd)/dist/bin/codegraph.js"
export CODEGRAPH_TELEMETRY=0 DO_NOT_TRACK=1 CODEGRAPH_NO_DAEMON=1
ON="$WORK/$NAME-on"; OFF="$WORK/$NAME-off"
rm -rf "$ON" "$OFF"; mkdir -p "$WORK"
rsync -a --exclude='.git' "$SRC/" "$ON/"; rsync -a --exclude='.git' "$SRC/" "$OFF/"
node "$CG" init "$ON" 2>&1 | grep -E "nodes,|Indexed"
CODEGRAPH_VALUE_REFS=0 node "$CG" init "$OFF" 2>&1 | grep -E "nodes,|Indexed"
OND="$ON/.codegraph/codegraph.db"; OFD="$OFF/.codegraph/codegraph.db"
echo "nodes on/off: $(sqlite3 "$OND" 'select count(*) from nodes') / $(sqlite3 "$OFD" 'select count(*) from nodes') (MUST MATCH)"
# PRECISE filter — do NOT use LIKE '%valueRef%' (it matches filenames like
# textModelValueReference.ts; see §7). Always: kind='references' AND the exact key.
F="kind='references' and metadata like '%\"valueRef\":true%'"
echo "value-ref edges: $(sqlite3 "$OND" "select count(*) from edges where $F")"
echo "=== top targets by same-file reader count ==="
sqlite3 -column "$OND" "select t.name, count(*) r, replace(t.file_path,'$ON/','') f from edges e join nodes t on e.target=t.id where e.$F group by e.target order by r desc limit 15;"
```
Run: `WORK=/tmp/cg-vr bash probe.sh /path/to/cloned-repo reponame`.
### 4.2 FP hunts (run against the ON db `$OND`, with `F` from above)
```bash
# (a) bundled/minified files among targets — the #1 FP source (the woff2 case):
sqlite3 "$OND" "select distinct t.file_path from edges e join nodes t on e.target=t.id where e.$F;" \
| while read -r f; do [ -f "$f" ] || continue; \
m=$(awk '{if(length>x)x=length}END{print x+0}' "$f"); [ "$m" -gt 300 ] && echo "MINIFIED? $m $f"; done
# (b) guard invariant — no surviving target re-declared in its file (adjust regex per language):
sqlite3 "$OND" "select distinct t.name, t.file_path from edges e join nodes t on e.target=t.id where e.$F limit 80;" \
| while IFS='|' read -r n f; do [ -f "$f" ] || continue; \
c=$(grep -cE "(const|let|var)[[:space:]]+$n\b" "$f"); [ "${c:-0}" -gt 1 ] && echo "LEAK $n x$c $f"; done
# (c) precision sample — eyeball reader->target pairs across the tree:
sqlite3 -column "$OND" "select s.name,'->',t.name from edges e join nodes s on e.source=s.id join nodes t on e.target=t.id where e.$F order by e.id desc limit 12;"
```
For each FP suspect, open the file and confirm whether the reader truly reads that file-scope
target. Cluster of FPs in one file → fix (extend a guard). One-off → record it, don't chase.
### 4.3 Impact-API delta (the headline) + agent A/B
Headline metric — value-refs turns a blind impact into a real one:
```bash
for s in SOME_CONST ANOTHER_CONST; do
printf "%-20s ON %s OFF %s\n" "$s" \
"$(node dist/bin/codegraph.js impact "$s" --path "$ON" 2>/dev/null | grep -oE '— [0-9]+ affected' | head -1)" \
"$(node dist/bin/codegraph.js impact "$s" --path "$OFF" 2>/dev/null | grep -oE '— [0-9]+ affected' | head -1)"
done
```
Pick targets from the probe's "top targets" list. Expect ON ≫ OFF (e.g. 1 → 90).
**Agent A/B** (optional per language — the finding below is size/language-independent, so the
deterministic probe + impact delta usually suffice). If you run it: two **fresh on/off
indexes**, pre-warm a `--no-watch` daemon per index, `claude -p` with **`--model sonnet
--effort high`**, ≥2 runs/arm. The pattern in `scripts/agent-eval/ab-new-vs-baseline.sh` is
the template **but it switches builds + re-indexes (no flag), which wipes a flag-specific
index — don't use it as-is for a flag A/B.** (Memories: `agent-eval-nested-attach`,
`agent-eval-targets-public-oss-only`.)
**The established A/B finding (don't re-derive):** across 12 runs on excalidraw both arms did
0 Read / 0 Grep — the agent answers impact questions in one call and reaches for
`codegraph_search`/`callers`, *not* `impact`/`explore`, so it often doesn't query the
value-ref edges at all. ON was never worse than OFF. **So: value-refs does NOT reduce agent
reads — the win is blast-radius correctness** (impact API / CodeGraph Pro's verdict engine).
---
## 5. Per-language checklist (the actual work)
### A. Where do "constants worth tracking" live? (decide FIRST)
The target gate now accepts **`file:`, `class:`, and `module:`** parents. Before anything:
- If the language puts shareable constants at **file/module scope** (TS/JS, Python module
consts, Go package vars, Rust module/impl `const`/`static`) → fits as-is; proceed.
- If constants live **inside a class/module** (Ruby — done) → the `class:`/`module:` gate now
covers them, BUT two things may need fixing first: (1) the extractor must actually *extract*
the class-internal constant as a node (the dispatch at the `variableTypes` branch skips
class-internal assignments — Ruby needed an exception for `constant`-LHS assignments); (2) the
reader-scan must match however the grammar represents a constant *reference* (Ruby uses
`constant` nodes, not `identifier`). See the Ruby block in the design doc.
- **Class-scope precision** uses a **file-wide** target map (one target per name per file), NOT
strict same-class matching — because lexical-scope languages (Ruby) let a nested class read an
enclosing class's constant, and strict matching would drop those valid reads. The only real FP
is the same constant name in *sibling* classes in one file (~1.7% of Ruby targets on rails);
valid code rarely hits it (a bare sibling-class constant is a NameError in Ruby).
- **Java/C#/Kotlin/Swift class-scope constants are DONE.** The gate now accepts `file:`/`class:`/
`module:`/**`struct:`/`enum:`** parents — the `struct:`/`enum:` widening was added for Swift, which
namespaces shared constants in `enum`/`struct` (`enum Constants { static let X }`). **Lesson for the
next class-scope language:** check the *parent kind* of a sample const (`select … substr(id…)`) — if
it's `struct:`/`enum:`/`interface:` and the gate doesn't list it, widen the gate (one line) or the
feature silently emits nothing despite the nodes existing.
- **Confirm the reader-scan matches the language's constant *reference* node type (the PHP lesson).**
The reader-scan in `flushValueRefs` matches `identifier` / `constant` / `name`. If the new language
represents a constant *read* as some other node type, the scan finds nothing and **no edges form**
even with targets correctly registered. PHP refs a const as a **`name`** node (bare `X`, and the
const half of `self::X` / `Foo::X`), which the scan missed until `name` was added. Dump a sample's
reader body (`scripts/agent-eval` or a quick `getParser` walk) and check the node type of a
constant reference *before* sweeping — a zero-edge sweep usually means this, not a target-gate bug.
### B. Confirm the declarator node type (for the shadow prune)
The shadow prune (in `flushValueRefs`) counts declarator names via a `switch (n.type)` over
declarator node types — a file only has its own grammar's nodes, so it's safe to list all
languages' types in one switch. **Add the new grammar's declarator types there**, with the
right way to pull the bound name(s). **Verify against the actual grammar** (don't trust this
table — confirm by parsing a sample). **This step is load-bearing:** if you skip it, the prune
silently does nothing for the new language and intra-file shadowing produces false positives
(this is exactly what happened on the first Go pass — see §5-Go below).
| Language | declarator node(s) | name extraction | status |
|---|---|---|---|
| TS/JS/tsx | `variable_declarator` | `namedChild(0)` | done |
| Go | `const_spec`, `var_spec`, `short_var_declaration` | spec → `namedChild(0)`; short-var → identifiers in the `left` field | **done** |
| Python | `assignment` | `left` field: identifier, or iterate a `pattern_list`/`tuple_pattern` | **done** |
| Rust | `const_item`, `static_item`, `let_declaration` | const/static → `name` field; let → `pattern` field | **done** |
| Ruby | `assignment` (LHS is a `constant` node) | already in the switch; Ruby can't local-shadow a constant, so the prune is effectively a no-op for it | **done** (class-scope) |
| Ruby | `assignment` with constant LHS (`CONST`) | LHS | to verify |
| C | `init_declarator` in a file-scope `declaration` | `cDeclaratorIdentifier` walks the `declarator` chain (init → pointer/array → identifier) | **done** |
| C++ | **attempted & reverted** — parse fidelity (see the C++ note in §2b) | — | reverted |
| Java | `variable_declarator` (field AND method-local) | `namedChild(0)` = name identifier — **already the TS/JS case**, no new wiring | **done** |
| C# | `variable_declarator` (field AND method-local) | same as Java — already in the switch | **done** |
| PHP | **none** | a `$var` local (`variable_name`) is a different namespace from a bare constant — a local can never shadow a constant, so the prune is a no-op and needs no PHP declarator | **done** (n/a) |
| Scala | `val_definition`, `var_definition` | `pattern` field (identifier) — catches an object/top-level val shadowed by a method-local `val` | **done** |
| Kotlin | `property_declaration` | `variable_declaration → simple_identifier` (and `bump` accepts `simple_identifier`) — catches an object/companion const shadowed by a method-local `val` | **done** |
| Swift | `property_declaration` | `<name> pattern → simple_identifier` (`firstSimpleIdentifier`) — the prune case resolves both Kotlin and Swift shapes; catches a static const shadowed by a method-local `let` | **done** |
| Dart | `static_final_declaration` (target) + `initialized_identifier` (field/`var`) + `initialized_variable_definition` (local) | each has a direct `identifier` child — catches a top-level/static const shadowed by a method-local `const` | **done** |
| Pascal | `declConst` (unit/class const = the target) + `declVar` (a local `var`) | `<name>` field — catches a unit `const X` shadowed by a function-local `const X` | **done** |
**The prune rule is `declarators > file-scope-node-count`, NOT `> 1`.** A name can be bound
twice *at file scope* legitimately — a **conditional module def** (`try: X = a; except: X = b`,
or `if cond: X = a else: X = b`). Those make N file-scope nodes AND N declarators, so they're
kept; a real local shadow makes declarators exceed file-scope nodes. Python forced this
refinement (try/except const defs are everywhere); it's strictly more correct for all
languages. `fileScopeValueCounts` (incremented in `captureValueRefScope`) tracks the file-scope
node count per name. Also: same-name value-ref edges are suppressed (`refName !== scope.name`),
since the two halves of a conditional def would otherwise cross-reference.
**Go was the worked example of "step B matters":** the first pass added `go` to
`VALUE_REF_LANGS` only, and a synthetic probe immediately showed a false positive —
`func withShadow() { TimeoutSeconds := 5; return TimeoutSeconds }` got edged to the package
`const TimeoutSeconds`, because the prune scanned `variable_declarator` (which Go doesn't
have). Fix: add Go's `const_spec`/`var_spec`/`short_var_declaration` to the switch. Note the
**precision-first tradeoff** this inherits from TS/JS — a shadowed target is dropped for the
*whole file*, so a legit reader elsewhere in that file loses its edge too. On the Go sweep
(gin/hugo/prometheus) this over-pruning was negligible (guard invariant clean, no LEAKs), so
it wasn't worth per-reader analysis — but re-check it per language.
### C. Confirm what kind the extractor assigns
`captureValueRefScope` keys off `kind ∈ {constant, variable}` for targets. Index a sample file
and check `select kind,name from nodes where file_path like '%sample%'` — confirm module-level
constants come out as `constant`/`variable` (not `field`, `property`, `import`, etc.). If they
come out as something else, adjust the target gate.
### D. Wire + sweep
1. Add the language string to `VALUE_REF_LANGS`.
2. `npm run build`.
3. Run §4.1 probe on **small / medium / large** public OSS repos (≥3 sizes). Prefer repos
with real config/constant/lookup-table modules (where the feature shines).
4. Run §4.2 FP hunts on each. Fix FP clusters (extend a guard); record singletons.
5. Run §4.3 impact delta on a few targets.
6. Add a **matrix row** to `value-reference-edges.md` (per language) and a **test** to
`__tests__/value-reference-edges.test.ts` (positive read + a shadow/negative case).
7. `npx vitest run __tests__/value-reference-edges.test.ts` and the full suite.
**Pass bar:** node count identical on/off at every size; precision samples clean (FP clusters
fixed); impact delta shows the blind→real radius win; full test suite green.
---
## 6. Git / PR workflow (how the prior ones were done)
- Branch off `main` (e.g. `feat/value-refs-<lang>`). This validation work has lived on
`feat/value-refs-validation`; a new language can extend it or take its own branch.
- A pure-validation change is **docs (+ a test)**; a precision fix is a focused **code** PR
(like #895). Keep code fixes separate from the doc/matrix update when practical.
- Commit-message trailer: `Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>`.
- PR body trailer: `🤖 Generated with [Claude Code](https://claude.com/claude-code)`.
- Merge is the **maintainer's call** — don't self-merge unless told. Branch protection needs
`gh pr merge --squash --admin` when authorised (memory: `gh-merge-needs-admin`).
- CHANGELOG: user-facing entries under `## [Unreleased]`; don't pre-create a version block.
---
## 7. Traps already hit (save yourself the time)
- **Probe false-match:** `metadata LIKE '%valueRef%'` matches *filenames* in other edges'
metadata (e.g. an `interface-impl` `calls` edge whose `registeredAt` is
`…/textModelValueReference.ts`). **Always** filter `kind='references' AND metadata LIKE
'%"valueRef":true%'`. This created a phantom "method target" FP on vscode that was pure
query noise.
- **`searchNodes` returns `SearchResult[]`** (`.node` wraps the `Node`) — in tests use
`.map(r => r.node)`. `getImpactRadius().nodes` is a **`Map`** — iterate `.values()`.
- **`CodeGraph.initSync(dir, opts)` ignores `opts`** — it takes only the path; the default
config indexes `.ts`/`.tsx`/`.js`. Don't rely on a passed `include`.
- **Node count must be identical on/off.** If it isn't, value-refs is (wrongly) creating nodes
— investigate before anything else.
- **Big repos:** indexing vscode (11.5k files) took ~2m and a ~1GB DB per arm; clean up
`/tmp` after (each on/off pair is hundreds of MB to >2GB).
- **require-bindings (CommonJS) are not FPs** — see §3. Don't "fix" them.
- **Don't over-engineer a guard for a gap that doesn't manifest** (e.g. param-only shadow):
evidence-driven only. The maintainer steered toward minimal, surgical fixes.
- **C macro-prefixed-prototype misparse (the C FP cluster):** an unknown leading macro
(`CURL_EXTERN`, `XXH_PUBLIC_API`) makes tree-sitter-c misparse a prototype `MACRO RetType
fn(args);` as a *declaration* whose declared "variable" is the bare return-type identifier
(`XXH_errorcode`), splitting `fn(args)` into a bogus expression. It mints one spurious type-named
global per prototype — then edged by every function of that type (redis `XXH_errorcode` 1→18).
These misparses *always* produce a **bare `identifier`** declarator (checked across
pointer/array/sized-return variants); real consts/tables always have an `init_declarator` and real
pointer/array globals their own declarator. Fix = **skip bare-`identifier` declarators** in the C
branch. The "extra" file-scope variable nodes also drop node-count vs an early pass — both arms
match, but don't be surprised the post-fix count is *lower*.
- **"Easy path" ≠ "nodes already exist."** The §2b table classifies by *scope*; it does not promise
the language's consts are extracted. C sat in the easy column yet emitted zero file-scope const
nodes. Run §5 step C (`select kind,name from nodes where file_path like '%sample%'`) on a sample
*first* — if the consts aren't there, you're doing the Ruby treatment, not the easy path.
- **Class consts may extract as `field` kind, not `constant` (Java/C#).** Step C must check the
*kind*, not just that a node exists: Java `static final` and C# `const`/`static readonly` came out
as `field`, which the value-ref target gate (`constant`/`variable` only) silently rejects — so the
feature emitted nothing despite the nodes being present. Fix = an `isConst` predicate on the
extractor (gated on the const modifiers) + a kind switch in `extractField` (scoped per-language so
other languages' fields stay `field`). Don't widen the *gate* to accept `field` — that would pull
in every mutable instance field as a target. And only the const *subset* converts: a Java instance
`final` or C# instance `readonly` is per-object state, must stay `field`.
- **A zero-edge sweep with correctly-registered targets = the reader-scan node type (the PHP trap).**
Targets can register perfectly (right kind, right scope) and *still* produce zero edges if the
reader-scan doesn't recognise how the language writes a constant *read*. PHP refs a const as a
**`name`** node, not `identifier`/`constant`, so the scan saw nothing until `name` was added to the
match. Before assuming a target-gate bug on a sparse/empty sweep, dump a reader body and check the
node type of a known constant reference. (Adding a ref node type to the scan is safe across
languages — `flushValueRefs` only runs for the value-ref set, and a file holds only its own
grammar's nodes; `name` is PHP-only among the current set.)
- **Same-file-only means cross-file-heavy languages yield less — that's correct, not a miss.** PHP
reads constants across files far more than within one (`Logger::DEBUG` everywhere), so laravel
(2,956 files) gave only 86 edges vs Ruby rails's 2,255. Don't chase it: cross-file value consumers
are out of scope for *every* language (would need import/scope resolution). Report the lower yield
honestly in the matrix rather than treating it as a bug to fix.
- **Some extractors emit parameters/fields as `variable` at the wrong scope — restrict to `constant`
(the Pascal trap).** Pascal's extractor emits function `const`/`var` parameters and class fields as
`variable` parented to the enclosing unit/class, so they pass the target gate and collapse to noisy
file-wide targets (`Dest`, `aItem` read "everywhere"). The genuine shared values were all `constant`
(`declConst`), so the fix is a one-line per-language restriction in `captureValueRefScope`: Pascal
targets `constant` only. Before trusting a new language's `variable` targets, sample them — if they're
parameters or instance fields rather than module/global state, restrict to `constant`. (A residual
tail can still leak: tree-sitter-pascal context-dependently misparses a `const` param in a complex
Delphi signature as a `declConst` — a small parse-fidelity FP, accepted as a documented caveat.)
- **A zero-edge sweep with targets present can be the READER side, not just the reader-scan node type
(the Dart trap).** Targets extracted fine, reader scopes registered, reader-scan node type correct —
and still zero edges, because Dart attaches a method **body as a next *sibling*** of the signature
node (which is what gets stored as the reader scope), so the scan walked only the signature subtree.
If a language's function/method body isn't a descendant of the node you register as the reader scope,
the scan won't see the reads — pull in the sibling/linked body. Check this when edges are zero but
both the targets and the reader nodes look right.
---
## 8. Reference
- Code: `src/extraction/tree-sitter.ts` (`VALUE_REF_LANGS`, `captureValueRefScope`,
`flushValueRefs`), `src/extraction/generated-detection.ts` (`isGeneratedFile`).
- Design + matrix: `docs/design/value-reference-edges.md`.
- Tests: `__tests__/value-reference-edges.test.ts`.
- PRs: **#895** (default-on + shadow prune), **#897** (TS/JS/tsx validation).
- Memories: `value-reference-edges-default-on`, `agent-eval-targets-public-oss-only`,
`agent-eval-nested-attach`, `gh-merge-needs-admin`, `impact-coverage-findings`.