From f45ea17288769b6e04916d4b0524fa33d57bcee4 Mon Sep 17 00:00:00 2001 From: Faisal-Fayaz <119831413+Faisal-Fayaz@users.noreply.github.com> Date: Sun, 4 Oct 2026 11:12:46 +0530 Subject: [PATCH] fix(extract): stop walking JSON Schema files as config manifests (#2255) `_is_config_json()` decides whether a `.json` file is worth AST-walking. It matches by filename first, then falls back to a root-key probe. `$schema` was in that probe's key set -- correctly, since configs legitimately carry it -- but it is also the *defining marker* of a JSON Schema, so every schema document satisfied the probe and got walked in full. That re-opened the keyword-node explosion #1224 closed for data JSON: one 160 KB contract contributed 362 nodes labelled with bare schema keywords (`description` x113, `type` x83, `$ref` x53, ...) plus ~12 schema-only communities, polluting clustering, god-node analysis and GRAPH_REPORT.md. Two changes, both ahead of the existing probe: - Drop `$ref` from the probe keys. A root-level `$ref` is a schema construct and never a config signal. - Return False when the root object carries `$schema` *plus* a schema-definition marker (`$defs`, `definitions`, or `$id`). This is what separates "this file IS a schema" from "this config POINTS AT a schema": the latter points `$schema` at its own tool's schema (biomejs.dev, docs.renovatebot.com) and never carries these markers. The blast radius is smaller than it looks. The filename branches run first, so biome.json / renovate.json / package.json / tsconfig.json never depended on the probe at all; only arbitrarily-named files did. I confirmed this against a control matrix before and after -- every config shape keeps byte-identical node counts. The root keys are now collected once into a set so the schema check and the config probe agree on the same read of the file, rather than walking the root twice. Documented boundary, pinned by a test: a hand-written schema carrying only `$schema` + `properties` (no `$defs`/`definitions`/`$id`) is still walked. Closing that gap needs a keyword-density heuristic, which would risk skipping a real config whose keys overlap schema vocabulary -- the worse error, since it drops structure silently. Left for a deliberate change rather than guessed at. Verified: - uv run pytest tests/test_json_schema_config.py -q (14 passed; 5 fail pre-fix) - uv run pytest tests/ -q -> 28 failed, 6262 passed, 101 skipped - baseline on a clean v0.9.75 checkout: 28 failed, 6248 passed, 101 skipped The 28 failures are byte-identical between the two (missing tree-sitter grammars for erlang/R/solidity/vbnet, plus ollama retry tests) and are diffed test-id-by-test-id, not compared by count. Passing delta is exactly +14. - uv run ruff check . -> clean - uv run pyright -> 636 errors, identical to the clean v0.9.75 baseline Co-Authored-By: Claude --- graphify/extractors/json_config.py | 54 +++++-- tests/test_json_schema_config.py | 227 +++++++++++++++++++++++++++++ 2 files changed, 272 insertions(+), 9 deletions(-) create mode 100644 tests/test_json_schema_config.py diff --git a/graphify/extractors/json_config.py b/graphify/extractors/json_config.py index e98666534..570692944 100644 --- a/graphify/extractors/json_config.py +++ b/graphify/extractors/json_config.py @@ -18,16 +18,43 @@ _CONFIG_JSON_NAMES = frozenset({ _CONFIG_JSON_KEYS = frozenset({ "dependencies", "devDependencies", "peerDependencies", "optionalDependencies", "bundleDependencies", "bundledDependencies", - "extends", "$ref", "$schema", "compilerOptions", + "extends", "$schema", "compilerOptions", }) +# Root-level keys that only a JSON Schema *document* carries. A schema that +# defines anything needs somewhere to put the definitions, and the spec's own +# keywords for that are `$defs` (2020-12) and `definitions` (draft-07/04); +# `$id` names the schema itself. Combined with `$schema` these separate "this +# file IS a schema" from "this config POINTS AT a schema" -- the latter points +# $schema at its own tool's schema (biomejs.dev, docs.renovatebot.com, ...), +# never at the meta-schema, and never carries these markers. +_SCHEMA_DEFINITION_KEYS = frozenset({"$defs", "definitions", "$id"}) + + def _is_config_json(path: Path, obj_node, source: bytes) -> bool: """True if a .json file is a recognized config/manifest worth AST-extracting. Matches by filename first (cheap), then falls back to a top-level key probe so arbitrarily-named config files (e.g. ``api.tsconfig.json``, ``foo.eslintrc.json``) are still picked up. Returns False for data JSON so it - is skipped by the structural pass (#1224).""" + is skipped by the structural pass (#1224). + + A JSON Schema is *not* a config manifest. `$schema` sits in the probe's key + set because configs legitimately carry it, but it is also the defining marker + of a schema -- so a schema document satisfied the probe and was walked in + full, re-opening the keyword-node explosion #1224 closed (#2255). One 160 KB + contract contributed 362 keyword nodes and ~12 schema-only communities. The + filename branches above run first, so `biome.json`/`renovate.json`/ + `package.json` never depended on the probe; the probe only decides the fate + of arbitrarily-named files, which is why narrowing it is low-risk. + + Documented boundary: a hand-written schema carrying `$schema` but no + `$defs`/`definitions`/`$id` is still walked. Catching it would need a + keyword-density heuristic, which would risk skipping a real config whose keys + happen to overlap schema vocabulary -- the worse error, since it drops + structure silently (#1224). Pinned by + ``tests/test_json_schema_config.py::test_minimal_schema_without_defs_or_id_is_still_walked``. + """ name = path.name.casefold() if name in _CONFIG_JSON_NAMES: return True @@ -35,7 +62,9 @@ def _is_config_json(path: Path, obj_node, source: bytes) -> bool: if name.endswith((".eslintrc.json", ".prettierrc.json", ".babelrc.json", "tsconfig.json", "jsconfig.json")): return True - # Top-level key probe: scan the root object's immediate keys (no deep walk). + # Collect the root object's immediate keys once (no deep walk), so the + # schema check below and the config probe agree on the same key set. + root_keys: set[str] = set() for top_key in obj_node.children: if top_key.type != "pair": continue @@ -43,9 +72,15 @@ def _is_config_json(path: Path, obj_node, source: bytes) -> bool: if key_node is None: continue kc = key_node.child_by_field_name("string_content") - text = _read_text(kc, source) if kc else _read_text(key_node, source).strip('"\'') - if text in _CONFIG_JSON_KEYS: - return True + root_keys.add(_read_text(kc, source) if kc + else _read_text(key_node, source).strip('"\'')) + # A file that declares the meta-schema AND defines something IS a schema. + if "$schema" in root_keys and (root_keys & _SCHEMA_DEFINITION_KEYS): + return False + # Top-level key probe. `$ref` is deliberately absent: a root-level `$ref` is + # a schema construct and never a config signal (#2255). + if root_keys & _CONFIG_JSON_KEYS: + return True return False def extract_json(path: Path) -> dict: @@ -53,9 +88,10 @@ def extract_json(path: Path) -> dict: Data-shaped JSON (eval fixtures, datasets, GeoJSON, API response dumps) is deliberately skipped — AST-walking it produced hundreds of orphan key-nodes - and duplicate communities that swamped real structure (#1224). Recognition - is by filename (package.json, tsconfig.json, …) or a top-level key probe - (dependencies / extends / $ref / $schema / compilerOptions).""" + and duplicate communities that swamped real structure (#1224). JSON Schema + documents are skipped for the same reason (#2255). Recognition is by filename + (package.json, tsconfig.json, …) or a top-level key probe (dependencies / + extends / $schema / compilerOptions).""" _JSON_MAX_BYTES = 1_048_576 # 1 MiB — skip large fixture dumps / GeoJSON blobs try: diff --git a/tests/test_json_schema_config.py b/tests/test_json_schema_config.py new file mode 100644 index 000000000..0575f6ca7 --- /dev/null +++ b/tests/test_json_schema_config.py @@ -0,0 +1,227 @@ +"""JSON Schema files must not be mistaken for config manifests. + +`_is_config_json()` decides whether a `.json` file is worth AST-walking. It +matches by filename first, then falls back to a root-key probe. But `$schema` +was in that probe's key set, and `$schema` is the *defining marker* of a JSON +Schema -- so every schema document satisfied the probe and got walked in full, +re-opening the keyword-node explosion #1224 closed for data JSON. + +One 160 KB contract contributed 362 nodes whose labels were bare schema +keywords (`description`, `type`, `$ref`, `required`, `properties`), plus ~12 +schema-only communities that polluted clustering, god-node analysis and +`GRAPH_REPORT.md` -- 13% of a 2,725-node graph (#2255). + +These tests pin both directions, because the fix removes keys from the probe and +the risk is over-correcting into skipping real configs: + +- a file that *is* a schema (`$schema` plus a schema-definition marker) is + skipped, not walked +- a root-level `$ref`, which is a schema construct and never a config signal, + no longer implies config +- every config shape that reaches the probe -- including configs that carry + `$schema` themselves -- is still extracted + +The filename branches run before the probe, so `biome.json`/`renovate.json`/ +`package.json` never depended on the probe at all; those are pinned explicitly +anyway, because a fix here must not be able to change them. +""" +from pathlib import Path + +from graphify.extractors.json_config import extract_json + +# A JSON Schema as a contract file actually looks like: `$id` + `$defs`. +SCHEMA_WITH_DEFS = '''\ +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "https://example.com/events-v1.json", + "title": "Event catalogue", + "$defs": { + "dataClassification": { + "description": "Handling class for the event payload.", + "type": "string", + "enum": ["Public", "Internal", "CUI"] + }, + "event": { + "description": "One domain event.", + "type": "object", + "required": ["id", "kind"], + "properties": { + "id": {"description": "Stable id.", "type": "string"}, + "kind": {"description": "Event kind.", "type": "string"}, + "seq": {"description": "Sequence.", "type": "integer", "minimum": 0}, + "tags": {"description": "Tags.", "type": "array", + "items": {"type": "string"}}, + "shape": {"description": "One shape.", "oneOf": [{"type": "string"}]} + }, + "additionalProperties": false + } + }, + "$ref": "#/$defs/event" +} +''' + +SCHEMA_WITH_LEGACY_DEFS = '''\ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "definitions": { + "point": {"description": "A point.", "type": "object"} + }, + "$ref": "#/definitions/point" +} +''' + +# A root-level $ref is a schema construct, never a config signal. +SCHEMA_REF_ONLY = '{"$ref": "https://example.com/other.json"}' + +# Real configs that carry $schema themselves. These point at their OWN tool's +# schema, not at the JSON Schema meta-schema -- which is exactly why $schema +# cannot be read as "this file is a schema". +BIOME = '''\ +{ + "$schema": "https://biomejs.dev/schemas/1.7.0/schema.json", + "files": {"include": ["src"]}, + "linter": {"enabled": true} +} +''' + +RENOVATE = '''\ +{ + "$schema": "https://docs.renovatebot.com/renovate-schema.json", + "extends": ["config:base"] +} +''' + +# Arbitrary filename -> only the root-key probe can catch these. +PROBE_EXTENDS = '{"extends": "./base.json", "rules": {"eqeqeq": "error"}}' +PROBE_COMPILER = '{"compilerOptions": {"strict": true, "target": "es2022"}}' +SUFFIX_TSCONFIG = '{"compilerOptions": {"strict": true}}' + + +def _write(tmp_path, name, body): + p = Path(tmp_path) / name + p.parent.mkdir(parents=True, exist_ok=True) + p.write_text(body) + return p + + +def _labels(result): + return [n["label"] for n in result["nodes"]] + + +def _is_skipped(result): + return result.get("skipped") == "data json (not a config/manifest)" + + +# --- the schema side: these must stop being walked ------------------------- + +def test_schema_with_defs_is_skipped_not_walked(tmp_path): + r = extract_json(_write(tmp_path, "events-v1.json", SCHEMA_WITH_DEFS)) + assert _is_skipped(r), r + assert r["nodes"] == [] + assert r["edges"] == [] + + +def test_schema_with_legacy_definitions_is_skipped(tmp_path): + # draft-07 schemas spell the same thing `definitions`. + r = extract_json(_write(tmp_path, "legacy.json", SCHEMA_WITH_LEGACY_DEFS)) + assert _is_skipped(r), r + assert r["nodes"] == [] + + +def test_schema_with_defs_in_subdirectory_is_skipped(tmp_path): + # The reporter's file lived at contracts/events-v1.json, i.e. the filename + # branch could never have matched it -- only the probe could. + r = extract_json(_write(tmp_path, "contracts/events-v1.json", SCHEMA_WITH_DEFS)) + assert _is_skipped(r), r + + +def test_root_level_ref_alone_is_skipped(tmp_path): + r = extract_json(_write(tmp_path, "ref.json", SCHEMA_REF_ONLY)) + assert _is_skipped(r), r + assert r["nodes"] == [] + + +def test_schema_produces_no_keyword_nodes(tmp_path): + # The actual symptom: bare schema keywords became graph nodes. + r = extract_json(_write(tmp_path, "events-v1.json", SCHEMA_WITH_DEFS)) + keywords = {"description", "type", "properties", "required", + "additionalProperties", "oneOf", "items", "enum"} + assert not keywords & set(_labels(r)), _labels(r) + + +# --- the config side: the fix must not over-correct ------------------------ + +def test_biome_json_with_its_own_schema_key_is_still_extracted(tmp_path): + # biome.json points $schema at biome's schema, not at the meta-schema. It is + # filename-matched, so it never depended on the probe -- pinned so this fix + # provably cannot affect it. + r = extract_json(_write(tmp_path, "biome.json", BIOME)) + assert not _is_skipped(r), r + assert "linter" in _labels(r) + + +def test_renovate_json_with_schema_and_extends_is_still_extracted(tmp_path): + r = extract_json(_write(tmp_path, "renovate.json", RENOVATE)) + assert not _is_skipped(r), r + assert "extends" in _labels(r) + + +def test_package_json_is_still_extracted(tmp_path): + r = extract_json(_write(tmp_path, "package.json", + '{"name": "x", "devDependencies": {"jest": "^29"}}')) + assert not _is_skipped(r), r + assert "devDependencies" in _labels(r) + + +def test_probe_still_recognises_extends_by_arbitrary_filename(tmp_path): + # `extends` stays in the probe; only $ref left. + r = extract_json(_write(tmp_path, "custom.config.json", PROBE_EXTENDS)) + assert not _is_skipped(r), r + assert "extends" in _labels(r) + + +def test_probe_still_recognises_compileroptions_by_arbitrary_filename(tmp_path): + r = extract_json(_write(tmp_path, "weird.json", PROBE_COMPILER)) + assert not _is_skipped(r), r + assert "compilerOptions" in _labels(r) + + +def test_compound_suffix_match_is_unaffected(tmp_path): + r = extract_json(_write(tmp_path, "api.tsconfig.json", SUFFIX_TSCONFIG)) + assert not _is_skipped(r), r + assert "compilerOptions" in _labels(r) + + +def test_dev_dependencies_probe_still_works(tmp_path): + r = extract_json(_write(tmp_path, "deps.json", PROBE_COMPILER)) + assert not _is_skipped(r), r + + +# --- documented boundary, pinned so a future change is deliberate ---------- + +def test_minimal_schema_without_defs_or_id_is_still_walked(tmp_path): + """A hand-written schema carrying only `$schema` + `properties` is still walked. + + This is a known, accepted residual: catching it needs a keyword-density + heuristic, which would risk the opposite error -- skipping a real config + whose keys happen to overlap schema vocabulary. Pinned here so that closing + the gap later is a deliberate change with a test, not an accident. + + `$defs`, `$id` and `definitions` are spec-defined and value-independent, so + they are the markers the fix relies on; this case has none of them. + """ + body = ('{"$schema": "https://json-schema.org/draft/2020-12/schema",' + ' "type": "object", "properties": {"a": {"type": "string"}}}') + r = extract_json(_write(tmp_path, "minimal.schema.json", body)) + assert not _is_skipped(r), r + assert "properties" in _labels(r) + + +# --- determinism ----------------------------------------------------------- + +def test_classification_is_deterministic_across_repeated_calls(tmp_path): + # Extraction must not depend on ordering or ambient state (CONTRIBUTING #3). + p = _write(tmp_path, "events-v1.json", SCHEMA_WITH_DEFS) + first = _labels(extract_json(p)) + for _ in range(5): + assert _labels(extract_json(p)) == first \ No newline at end of file