Skip to content

merge-batch-graphs.py drops every tested_by edge whose production endpoint is a non-file: node (_file_node_path is file:-only) #664

Description

@LeeGwanHui

Summary

merge-batch-graphs.py's tested_by linker classifies nodes via _file_node_path(), which returns None for any node whose id doesn't start with file:. Because link_tests() builds its test/prod classification map only from nodes where that helper returns non-None, every tested_by edge whose production endpoint is a config: / document: / schema: / service: / pipeline: / table: / resource: / endpoint: node falls through to the else: dropped += 1 branch and is discarded as an "orphan endpoint" — even when both endpoints exist as valid nodes in the same merge.

This is the same hardcoded-file:-prefix assumption as Bug 3 in #366, but at a different site: #366 Bug 3 is recover_imports_from_scan(); this is _file_node_path() → link_tests(). It is also distinct from #595, which is about the test endpoint failing is_test_path(); here the production endpoint never enters classification at all.

Found on 2.9.4 (plugin cache, skills/understand/merge-batch-graphs.py).

Root cause

# merge-batch-graphs.py L569-577
def _file_node_path(node: dict[str, Any]) -> str | None:
    """Return the relative project path for a `file:`-prefixed node, else None."""
    nid = node.get("id", "")
    if not isinstance(nid, str) or not nid.startswith("file:"):   # <-- file: only
        return None
    ...
# link_tests(), L656-663 — only file: nodes ever get classified
for node in nodes_by_id.values():
    path = _file_node_path(node)
    if path is None:
        continue                       # config:/document:/schema:… skipped entirely
    ...
    node_id_to_classification[node["id"]] = "test" | "prod"
# L697-706 — unclassified endpoint => dropped
src_class = node_id_to_classification.get(src)   # None for config: nodes
tgt_class = node_id_to_classification.get(tgt)
if (src_class, tgt_class) == ("prod", "test"): ...
elif (src_class, tgt_class) == ("test", "prod"): ...
else:
    dropped += 1
    continue

This contradicts the skill's own contract. SKILL.md declares 9 file-level node types (L824-836) and its Phase 6 validator (L641) uses exactly that set:

const fileLevelTypes = new Set(['file', 'config', 'document', 'service', 'pipeline', 'table', 'schema', 'resource', 'endpoint']);

So the validator treats 9 types as file-level while the tested_by linker treats 1.

Minimal reproduction

.ua/intermediate/batch-1.json:

{
 "nodes": [
  {"id":"config:docs/app.config.json","type":"config","name":"app.config.json","filePath":"docs/app.config.json","summary":"s","tags":["config"]},
  {"id":"file:src/app.py","type":"file","name":"app.py","filePath":"src/app.py","summary":"s","tags":["code"]},
  {"id":"file:tests/test_config.py","type":"file","name":"test_config.py","filePath":"tests/test_config.py","summary":"s","tags":["test"]},
  {"id":"file:tests/test_app.py","type":"file","name":"test_app.py","filePath":"tests/test_app.py","summary":"s","tags":["test"]}
 ],
 "edges": [
  {"source":"config:docs/app.config.json","target":"file:tests/test_config.py","type":"tested_by","weight":0.5},
  {"source":"file:src/app.py","target":"file:tests/test_app.py","type":"tested_by","weight":0.5}
 ]
}
$ python merge-batch-graphs.py <repo>
Input: 4 nodes, 2 edges
Fixed (1 corrections):
     1 × tested_by edges dropped (orphan endpoint or test↔test / prod↔prod pair)
Output: 4 nodes, 1 edges

Expected: both tested_by edges survive; both production nodes get the tested tag.
Actual: only file:src/app.py → file:tests/test_app.py survives. The config: edge is dropped and config:docs/app.config.json is never tagged tested, despite both endpoints being present, correctly typed, and correctly directed.

Real-world impact

Dogfooded on a 65-file repo (psychology research DB in YAML/JSON + a build-free static PWA; 6 pytest modules + 3 node --test modules). Its deployed data files are legitimately typed config: by the file-analyzers, and its tests genuinely assert against them.

run tested_by dropped production nodes tagged tested
full build (64 files) 14 4
incremental (65 files) 18 1

Everything lost was real coverage that the test sources verify literally:

  • all 10 × config:docs/courses/<slug>.json → file:tests/test_course_data.py — that test does COURSES.glob("*.json") over exactly those files
  • config:docs/courses/index.json → file:tests/test_archive_index.py — that test opens index.json directly
  • config:docs/manifest.webmanifest → file:tests/test_manifest.py — that test reads docs/manifest.webmanifest and resolves its URLs

After restoring by hand, tested tags went 4 → 16 on the full build, and tested_by edges ended at 23 on the incremental. Pass 2 (path-convention supplement) does not rescue these: it emits 0 edges here, because the production side is config:-typed and so is invisible to it as well.

The loss recurs on every full merge, and (per the pruning behavior) on incrementals that touch those files.

Suggested fix

Classify by node["type"] against the file-level set the skill already documents, rather than by id prefix:

_FILE_LEVEL_TYPES = frozenset({
    "file", "config", "document", "service",
    "pipeline", "table", "schema", "resource", "endpoint",
})

def _file_node_path(node: dict[str, Any]) -> str | None:
    """Return the relative project path for a file-level node, else None."""
    if node.get("type") not in _FILE_LEVEL_TYPES:
        return None
    fp = node.get("filePath")
    if isinstance(fp, str) and fp:
        return fp
    nid = node.get("id", "")
    if isinstance(nid, str) and ":" in nid:
        return nid.split(":", 1)[1]
    return None

Keying on type rather than the id prefix also avoids a wrinkle in the id-suffix fallback: table: and endpoint: ids carry a trailing :<name>, so stripping only the prefix would not yield a clean path for them. filePath is normally present, and this ordering prefers it.

Two things worth checking when applying this:

  1. Widening classification means more nodes become "prod", which enlarges the candidate pool for the Pass 2 path-convention linker. That should only add correct supplements, but it is a behavior change beyond Pass 1 and deserves a test.
  2. The same file:-only assumption may exist at other sites; Incremental update: 3 edge-fidelity bugs (rename orphans, inbound-edge pruning, importMap recovery skips non-file: nodes) #366 Bug 3 documents one in recover_imports_from_scan(). A grep for startswith("file:") / f"file:{...}" would be worth doing as a sweep rather than patching site by site.

Related

— 🤖 Claude, on behalf of @LeeGwanHui

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions