Repository navigation
feat(scip): ingest real SCIP indexes (protobuf), not only SCIP-style JSON - #5070
LHopitalOne wants to merge 3 commits into
Conversation
…JSON
scip_ingest.py could read only a simplified, LLM-style SCIP JSON shape; the
index.scip files that scip-python, scip-typescript, rust-analyzer and other
indexers write were out of reach. This adds:
- graphify/scip_index.py: decode_scip_index() reads the SCIP protobuf wire
format directly (no new dependency) into the JSON shape `scip print --json`
uses. It reads the deprecated packed `range` and the typed
single_line_range / multi_line_range of SCIP >= 0.4 (typed wins), skips
unknown fields, and raises ScipDecodeError on a damaged index instead of
guessing. symbol_display_name() labels a symbol from its last descriptor.
- scip_ingest.ingest_scip_index(path | bytes): decodes, moves each document's
occurrences under the symbols it defines (definition first, zero-based SCIP
lines turned into Graphify's one-based ones), leaves out document-local
symbols, labels symbols without a display_name ('Square', 'area()'), then
runs the existing ingest_scip_json passes. Oversized, unreadable or malformed
input returns {"nodes": [], "edges": [], "error": ...} like
extract_mcp_config, honouring GRAPHIFY_MAX_GRAPH_BYTES.
Reference occurrences are not turned into edges yet: without enclosing_range
an occurrence does not say which symbol it belongs to, and guessing would add
relationships the index does not state.
Tests cover both range encodings, proto3 zero omission, unknown fields,
malformed input, the error results and a real index written by
scip-python 0.6.8 (tests/fixtures/scip). The existing ingest_scip_json tests
are unchanged and pass.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks for the pull request, @LHopitalOne. A maintainer will review it soon. Want to talk it through while it is in review? Come join us on our Discord server. For longer-form discussion there is also GitHub Discussions. A couple of things that speed up review: make sure the test suite passes on Python 3.10 and 3.13, and that the change keeps extraction deterministic. |
…ypes decode_scip_index now returns ScipIndex, a TypedDict tree (ScipDocument, ScipOccurrence, ScipSymbol, ScipRelationship, ScipMetadata) instead of dict[str, Any], and ingest_scip_index returns ScipIngestResult. Raw field values are typed int | memoryview | None, and reading one as the wrong kind raises ScipDecodeError: before, a string field written as a varint would have been decoded as zero bytes instead of rejected. Same nodes and edges as before on the test fixtures and on real indexes of getsentry/sentry, relay and snuba. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
![graphify-labs[bot]](https://avatars.githubusercontent.com/in/4356270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 5 advisory finding(s) below merit a look before merge.
Not checked: tests were not run; no formal proof of the changed code.
Graphify review — findings
Adds a dependency-free protobuf reader, decode_scip_index, that turns a binary index.scip into the same JSON-shaped dict scip print --json emits, skipping unknown fields and preferring the typed single/multi-line ranges over the deprecated packed range. It raises ScipDecodeError on truncated varints, overlong lengths and unknown wire types, but the decoder trusts field wire types and sizes, so a mistyped field or oversized length can still surface as a raw TypeError or force a huge allocation instead of that clean error. symbol_display_name derives a short label from a symbol's last descriptor, and the ingest module in scip_ingest.py now accepts a real index and maps its relationships to graph edges, with fixtures and tests covering the shapes, malformed input and relationship edges.
Worth a look
- Varint-typed string field makes _text allocate an attacker-chosen number of bytes —
graphify/scip_index.py· Escalate · high- NOT verified (no proof, no reproducing execution) — needs human review
- Unbounded varint length allows quadratic CPU DoS —
graphify/scip_index.py:315· Escalate · medium- NOT verified (no proof, no reproducing execution) — needs human review
- Known field with an unexpected wire type raises TypeError, not ScipDecodeError —
graphify/scip_index.py:192· Escalate · medium- NOT verified (no proof, no reproducing execution) — needs human review
- Valid protobuf with mistyped fields escapes the error contract and raises TypeError —
graphify/scip_ingest.py:210· Escalate · medium- NOT verified (no proof, no reproducing execution) — needs human review
- File size check can be bypassed before an unbounded read —
graphify/scip_ingest.py:169· Escalate · medium- NOT verified (no proof, no reproducing execution) — needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 169 functions depend on the 84 functions this change touches.
Health — this change adds coupling hotspots:
- worse:
ingest_scip_json()— 77 callers, 4 callees - new:
decode_scip_index()— 12 callers, 5 callees - new:
_shapes_index()— 5 callers, 6 callees - new:
ingest_scip_index()— 7 callers, 4 callees - new:
test_decode_skips_unknown_and_fixed_width_fields()— 0 callers, 7 callees - new:
test_decode_typed_range_wins_over_the_deprecated_packed_one()— 0 callers, 6 callees
Verification — 169 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 169 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
527 of 527 test file(s) selected (100%) via static blast radius.
Escalated to a full run for safety (
noncode-change,uncovered-code) — the selection is not trustworthy on its own (see below). CI should run the whole suite.
tests/pi.test.ts— full-run-safetytests/test_4874_jvm_resolution.py— full-run-safetytests/test_4909_reference_binding_scale.py— full-run-safetytests/test_4978_reconcile_cost.py— full-run-safetytests/test_4992_ts_import_types.py— full-run-safetytests/test_affected_cli.py— full-run-safetytests/test_affected_defines.py— full-run-safetytests/test_affected_member_seed.py— full-run-safetytests/test_affected_overrides.py— full-run-safetytests/test_affected_path_file_node.py— full-run-safetytests/test_affected_seed_reference_forms.py— full-run-safetytests/test_agents_platform.py— full-run-safetytests/test_alias_folding.py— full-run-safetytests/test_analyze.py— full-run-safetytests/test_anthropic_custom_endpoint.py— full-run-safetytests/test_antigravity_install.py— full-run-safetytests/test_apm_fallback_version.py— full-run-safetytests/test_architecture_doc.py— full-run-safetytests/test_astro_extraction.py— full-run-safetytests/test_astro_import_ids.py— full-run-safetytests/test_atomic_canvas_export.py— full-run-safetytests/test_atomic_version_stamp.py— full-run-safetytests/test_atomic_writes.py— full-run-safetytests/test_backend_env_isolation.py— full-run-safetytests/test_backend_extras.py— full-run-safetytests/test_backend_opt_in.py— full-run-safetytests/test_benchmark.py— full-run-safetytests/test_benchmark_raw_graph.py— full-run-safetytests/test_benchmark_real_query.py— full-run-safetytests/test_binary_media_words.py— full-run-safetytests/test_blade_extractor.py— full-run-safetytests/test_build.py— full-run-safetytests/test_build_located_semantic_identity.py— full-run-safetytests/test_build_merge_dedup_scope.py— full-run-safetytests/test_build_merge_ghost_fold_4955.py— full-run-safetytests/test_build_merge_hyperedges_and_prune.py— full-run-safetytests/test_build_merge_shrink_guard.py— full-run-safetytests/test_builtin_global_type_refs.py— full-run-safetytests/test_c_include_resolution.py— full-run-safetytests/test_cache.py— full-run-safetytests/test_cache_long_paths.py— full-run-safetytests/test_cache_stale_import_target.py— full-run-safetytests/test_cache_windows_case_4925.py— full-run-safetytests/test_callflow_html.py— full-run-safetytests/test_cargo_introspect.py— full-run-safetytests/test_cargo_missing_manifest.py— full-run-safetytests/test_carried_hyperedge_remap.py— full-run-safetytests/test_case_only_decl_ids_4904.py— full-run-safetytests/test_case_sensitive_resolution.py— full-run-safetytests/test_charmap_encoding.py— full-run-safety- … and 477 more
non-code file(s) changed (
tests/fixtures/scip/shapes.scip) → running the full suite for safety (a code graph can't see config/fixture/data deps)
changed code file(s) with no mapped test (
tests/fixtures/scip/shapes.py) — a coverage gap or a missing link — running the full suite rather than only the selected tests
Selection is safe under the controlled-regression assumption; always-run tests + a periodic full run are the backstops. Advisory — it never changes the check verdict.
Risky patterns (advisory)
- medium
no-unbounded-fetchingraphify/scip_ingest.py:176: unbounded read (no limit/bound): data = handle.read()
Docs that may be stale (advisory)
BENCHMARKS.md§ Judge and grading (lines 68-84): references changed symbolstotalCHANGELOG.md§ 0.9.101 (2026-11-02) (lines 58-81): references changed symbols__init__CHANGELOG.md§ 0.9.85 (2026-10-10) (lines 412-434): references changed symbolsShapeCHANGELOG.md§ 0.9.84 (2026-10-10) (lines 435-448): references changed symbols__init__CHANGELOG.md§ 0.9.77 (2026-10-05) (lines 529-548): references changed symbols__init__CHANGELOG.md§ 0.9.58 (2026-09-10) (lines 781-798): references changed symbols__init__CHANGELOG.md§ 0.9.44 (2026-08-15) (lines 978-990): references changed symbolsareaCHANGELOG.md§ 0.9.43 (2026-08-14) (lines 991-999): references changed symbolsareaCHANGELOG.md§ 0.9.42 (2026-08-13) (lines 1000-1018): references changed symbols__init__CHANGELOG.md§ 0.9.23 (2026-07-21) (lines 1213-1222): references changed symbols__init__
…and 10 more.
· 5 grounded finding(s) anchored inline below; 1 more finding(s) on lines outside this diff (see the check run).
| 57: "TypeFamily", | ||
| 58: "TypeParameter", | ||
| 59: "Union", | ||
| 60: "Value", |
There was a problem hiding this comment.
decode_scip_index()
12 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
| ScipDecodeError, | ||
| ScipIndex, | ||
| ScipOccurrence, | ||
| ScipSymbol, |
There was a problem hiding this comment.
ingest_scip_json()
77 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
| assert [o["range"] for o in occurrences] == [[0, 4, 9], [2, 1, 5, 0]] | ||
|
|
||
|
|
||
| def test_decode_typed_range_wins_over_the_deprecated_packed_one() -> None: |
There was a problem hiding this comment.
test_decode_typed_range_wins_over_the_deprecated_packed_one()
fans out to 6 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
| assert decoded["documents"] == [] | ||
|
|
||
|
|
||
| def test_decode_skips_unknown_and_fixed_width_fields() -> None: |
There was a problem hiding this comment.
test_decode_skips_unknown_and_fixed_width_fields()
fans out to 7 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
| # --------------------------------------------------------------------------- | ||
| # ingest_scip_index | ||
| # --------------------------------------------------------------------------- | ||
|
|
There was a problem hiding this comment.
_shapes_index()
fans out to 6 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
Two findings from the review bot, both reproduced first: - A varint longer than protobuf's 10 bytes made _varint keep shifting a growing int, so decoding slowed quadratically (320 KB of 0xff took 9 s). It now raises ScipDecodeError after 10 bytes. - ingest_scip_index checked os.path.getsize() and then read the whole file, so a FIFO or device that reports size 0 (e.g. /dev/zero) was read without end. It now reads at most GRAPHIFY_MAX_GRAPH_BYTES + 1 bytes and rejects anything longer. The bot's other three findings (a string field holding a varint, and wrong wire types raising TypeError) were fixed by the previous commit. A fuzz run of 30,000 corrupted indexes raised nothing but ScipDecodeError; slowest input 49 ms. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
What does this PR do?
Follows up on the split you asked for when closing #783: SCIP ingest (
scip_ingest.py) as its own focused PR,on current
v8.scip_ingest.ingest_scip_jsonreads a simplified, LLM-style SCIP JSON shape (its docstring says so). Theindex.scipfiles that real indexers write (scip-python, scip-typescript, scip-java, rust-analyzer, ...) areprotobuf, keep occurrences on the document rather than on each symbol, and use zero-based lines, so they could not be
ingested. This PR makes them ingestible:
graphify/scip_index.py(new):decode_scip_index(data)reads the SCIP protobuf wire format directly, with nonew dependency, into the JSON shape
scip print --jsonprints, typed asScipIndex(a TypedDict tree:ScipDocument,ScipOccurrence,ScipSymbol,ScipRelationship,ScipMetadata).rangeand the typedsingle_line_range/multi_line_rangethat SCIP 0.4+writes (typed wins, as scip.proto specifies). Handles proto3's omitted zero fields.
ScipDecodeErroron a damaged index instead of guessing (truncated varint, a length past its message, anunknown wire type, a number where scip.proto has a string or message, or the reverse).
symbol_display_name()labels a symbol from its last descriptor:Square,area(), matching how the ASTextractors label callables.
scip_ingest.ingest_scip_index(path | bytes)(new entry point): decodes, then maps onto the existing shape andruns the existing
ingest_scip_jsonpasses unchanged, returningScipIngestResult:source_locationis its definition line;
L1is the first line);local 12) are left out: no identity outside their file, no name;{"nodes": [], "edges": [], "error": ...}likeextract_mcp_config, and honoursGRAPHIFY_MAX_GRAPH_BYTES.What it deliberately does not do (yet)
enclosing_range, and most do not. Turning references into edges would mean guessing the enclosing symbol, so thisPR leaves them out rather than add relationships the index does not state.
.scipfiles are not picked up bydetect/extracthere; that touches binary-filehandling and the caches, so it seemed better as a separate PR. Happy to do it next in whichever form you prefer
(auto-detecting
*.scipin the corpus, or an explicit--scip index.scipflag).Type of change
Verification & Invariants
Invariants: the existing
ingest_scip_jsonpath and its output are untouched (the new entry point only adds a decodeand reshape step in front of it); a node's
source_locationis its definition line, one-based; a damaged or oversizedindex yields an error result, never an exception or a partial graph; the same index always gives the same graph.
index.scipcould not be read at all).How was this tested?
tests/test_scip_index.py(29 tests): both range encodings, typed range precedence, proto3 zero omission, unpackedrepeated ranges, symbol information and relationship flags, external symbols, unknown and fixed-width fields,
malformed input (including wrong wire types), labels for Python / Rust / TypeScript symbols, definition lines, relationship edges, path vs bytes,
the error results, determinism.
tests/fixtures/scip/shapes.scip(3.7 KB), written by scip-python 0.6.8 from the 18-lineshapes.pynext to it (the command is in the test). It checks the nodes land on their definition lines and thatscip-python's two
is_implementationrelationships (Square→Shape,Square.area→Shape.area) becomescip_impledges.tests/test_scip_ingest.pyis unchanged and passes.test_skillgen.py,test_skill_no_backtick_substitution.py) failed because the commits they audit were missing. With full history allbut one pass. The remaining one,
test_skill_no_backtick_substitution.py::test_control_a_backticked_unknown_command_is_caught, fails the same way onthe untouched
v8base (16218bf) on my machine, so it is not from this change.pip-audit --strictreports advisories forpypdf6.21.2, which comes from the committeduv.lock. This PR does nottouch the lock, so the audit job will likely flag it on
v8as well.Also run on real indexes of large repositories, outside the test suite:
(rust-analyzer writes no relationships when run without build scripts, hence 0 edges for relay.)
Graphify-specific checklist
uv run python -m tools.skillgen --bless) when changing their source fragments. (n/a: no skill fragments changed)GRAPHIFY_MAX_GRAPH_BYTESsize cap)🤖 Generated with Claude Code