Skip to content

fix(extract): parse only Svelte script blocks in the AST pass (#3928) - #3984

Closed
Agnik47 wants to merge 2 commits into
Graphify-Labs:v8from
Agnik47:fix/3928-svelte-script-masking
Closed

Agnik47 wants to merge 2 commits into
Graphify-Labs:v8from
Agnik47:fix/3928-svelte-script-masking

Conversation

@Agnik47

@Agnik47 Agnik47 commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes #3928. extract_svelte called _extract_generic(path, _JS_CONFIG) on the raw .svelte file. The markup is not JavaScript, so tree-sitter produced a top-level ERROR at line 1 and never reached the declarations inside <script> — every component was reported as "had syntax errors and may be partially extracted", and only imports survived, via the existing regex rescue pass.

This is the same failure extract_astro had, fixed in #3902 (f9a6893), and that extract_vue already avoids. This change gives Svelte the same treatment: blank everything outside the <script> bodies (keeping \r/\n, so AST locations still match the original file) and parse only those bodies.

Details:

  • Grammar follows the first block's lang: js/jsx → JS, ts or unset → TS (a superset of JS, and Svelte's own default), mirroring extract_vue.
  • Both script blocks are parsed — the instance block plus Svelte 5 <script module> / Svelte 4 <script context="module">.
  • Non-JS type scripts are blanked (application/ld+json and friends): their bodies are data, not statements, and would only add parse errors — same rule as the Astro masker.
  • A script region is terminated in place of the following <, so two blocks on one line do not run together into a single statement (the case test_extract_astro_scripts_on_one_line_do_not_merge covers for Astro).
  • The regex import rescue is unchanged. It still covers template-layer dynamic imports ({#await import('./X.svelte')}), which the AST pass cannot see. An import that both passes now resolve yields the same (source, target, relation) edge twice, which build.dedupe_edges collapses — identical to the Astro path's behaviour today (verified: both emit 2, both collapse to 1).

_svelte_mask_non_script mirrors _astro_mask_non_script, and the two share the script-tag and non-JS-type patterns instead of duplicating them (_ASTRO_SCRIPT_RE / _ASTRO_NON_JS_TYPE_RE now alias the shared constants — no behaviour change). Happy to drop that bit of sharing and keep the Astro lines untouched if you'd rather the diff stayed strictly inside extract_svelte.

Also covers the Svelte half of #3942: because the markup is blanked, a template {#if} no longer mints a spurious if() function node. That issue's other half (export type *) is not touched here.

Measured

The issue's minimal repro, graphify extract . --code-only --no-cluster:

files flagged with syntax errors result
before 1 of 1 2 nodes, 1 edges — bump() missing
after 0 of 1 3 nodes, 3 edges — bump() present

A small SvelteKit-shaped corpus (runes $state/$derived/$effect/$props, {#snippet}/{@render}, context="module", TS type aliases, a ld+json block, <style>):

files with parse errors nodes edges
before 3 of 3 7 6
after 0 of 3 14 17

Type of change

  • Bug fix
  • New feature
  • Documentation
  • Tests or CI
  • Refactor
  • Security fix

Verification & Invariants

The invariant: deterministic AST extraction must not silently drop symbols a file really declares. A .svelte file's <script> body is ordinary JS/TS; feeding the surrounding markup to the JS grammar turned a fully extractable file into a line-1 parse error plus a near-empty node set, and the only signal was a warning that is easy to miss in a large run.

Nothing persisted is invalidated: node ids are unchanged (they derive from the path and symbol name, and masking preserves offsets, so source_location values still point at the original lines). The graph gains the symbols that were previously dropped, which is a growth, not a shrink, so the anti-shrink guard is not implicated.

  • Read the CONTRIBUTING.md guide.
  • Reproduced the issue and identified the invariant.
  • Made the smallest fix necessary.
  • Added a regression test (if bug fix) or isolated boundary test.
  • Kept the PR description synchronized with the final implementation.
  • Documented any limitations / unsupported cases explicitly.

Limitations, stated explicitly:

  • An unterminated <script> (no closing tag) extracts nothing. It matched nothing before either (zero symbols, parse error), so this is not a regression; it is malformed Svelte and the tag regex requires a close.
  • lang="jsx" maps to the JS grammar for symmetry with extract_vue; Svelte itself does not support JSX, so this should never fire in practice.
  • Markup-layer expressions are deliberately not code to the AST pass. Dynamic imports there are still recovered by regex; nothing else in the template is extracted.

How was this tested?

# the issue's own repro, before and after
graphify extract . --code-only --no-cluster

# new regression tests (8)
uv run --frozen pytest tests/test_svelte_extraction.py -q          # 8 passed

# regression semantics: with the extract.py change stashed, 4 of the 8 fail
git stash push graphify/extract.py && uv run --frozen pytest tests/test_svelte_extraction.py -q
#   4 failed, 3 passed   (the import tests pass either way — they guard against
#   the masking costing imports the rescue pass already recovered)

# neighbours of this code path
uv run --frozen pytest tests/test_astro_extraction.py tests/test_astro_import_ids.py \
  tests/test_js_import_resolution.py tests/test_import_extension_resolution.py \
  tests/test_partial_extraction_warning.py tests/test_languages.py tests/test_extract.py -q

# full suite, lint, types
uv run --frozen pytest tests/ -q
uv run --frozen ruff check .          # clean
uv run --frozen pyright graphify/extract.py

Environment: Windows 11, Python 3.12 (uv-managed), uv sync --all-extras --frozen.

Full suite on this branch: 52 failed, 6125 passed, 62 skipped. To be sure none of that is mine, I ran the full suite a second time on a clean worktree checked out at the merge base (ef4450d) and diffed the two failure lists. No test fails on this branch that passes at the merge base, with one apparent exception that is a property of the checkout path, not of the change:

  • test_install.py::test_codex_hook_command_is_a_real_cli_subcommand fails in my main checkout and passes in the worktree. The reason is that my checkout lives under D:\Download2\Open Source Con\…: the test does command.split() on the registered hook command and asserts parts[1] is a known subcommand, so the space in the directory name makes it read 'Source'. The worktree happens to sit under a space-free temp path. It fails identically on unmodified code at the same path, so it is unrelated to this PR (and is the same class of problem as Git hooks can never resolve the interpreter when Python is under a path with a space (Windows) #2581).

Everything else failing is pre-existing and nowhere near extraction — test_hooks.py (12), test_skillgen.py (11), test_non_regular_files.py (5), test_extract.py parallel-fallback (5), skillgen path-injection (3), test_install.py (3), plus singles in watch, uninstall scope, atomic writes, terraform, and unicode wikilink normalization. No Svelte or Astro test is among them.

Cross-platform notes (CONTRIBUTING #13): the masker preserves \r and \n, so CRLF files keep correct line numbers; verified explicitly, along with a UTF-8 BOM file, an empty file, a markup-only file, a style-only file, two scripts on one line, and a Vue-3.3-style attribute containing > (generic="T extends Record<string, unknown>"). All extract cleanly with no parse errors.

Graphify-specific checklist

  • I updated generated skill artifacts (uv run python -m tools.skillgen --bless) when changing their source fragments. — n/a, no fragment touched.
  • I confirmed that AST/structural extraction remains deterministic (no ambient state dependencies like ENV variables). — the masker is a pure function of the file's bytes.
  • I reviewed changes for security implications (no unsafe interpolation into shell/Python).
  • I confirmed no API keys or local-only graph data are included.
  • (If applicable) I disclosed AI authorship in my commit messages.

🤖 Generated with Claude Code

extract_svelte fed the whole .svelte file to the JS grammar. The template
is not JS, so every component reported a syntax error at line 1 and no
symbol declared inside <script> was ever reached — only imports survived,
via the regex rescue pass.

Blank everything outside the <script> bodies (keeping newlines so
locations still match) and parse with the grammar the first block's lang
implies: js/jsx -> JS, ts or unset -> TS, a superset of JS and Svelte's
own default. <script module> and <script context="module"> are parsed
alongside the instance block. Scripts with a non-JS type
(application/ld+json, ...) are blanked too, and a script region is
terminated in place of the following `<` so two blocks on one line do not
run together. The existing regex import rescue is unchanged; an import
both passes resolve yields one edge after build.dedupe_edges, as on the
Astro path.

The masker mirrors _astro_mask_non_script and shares the script-tag and
non-JS-type patterns with it instead of duplicating them.

Fixes Graphify-Labs#3928. Covers the Svelte half of Graphify-Labs#3942 (markup is blanked, so a
template {#if} no longer mints a spurious if() node).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@Agnik47
Agnik47 requested a review from safishamsi as a code owner October 2, 2026 05:58
@github-actions

github-actions Bot commented Oct 2, 2026

Copy link
Copy Markdown

Thanks for the pull request, @Agnik47. 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.

@graphify-labs graphify-labs Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Graphify reviewed this change.

Worth a look — the grounded gate found no coupling regressions or blocking issues, but 1 advisory finding(s) below merit a look before merge.

Formal verification. 1 change(s) alter behavior, breaking input(s) attached. PR-changed functions: 1/1 verified (0 proven, 0 may-equivalent, 1 distinguished) · 0 not verified.

Behavior changes: extract\_svelte changes behavior, here is the input that shows it.

The verifier found a concrete input on which extract\_svelte behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.

Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.

Evidence: On input \{"path":"\_\_import\_\_\('pathlib'\)\.Path\('missing/deeper\.txt'\)"\}, the old code produced \{'nodes': \[\], 'edges': \[\], 'error': "\[Errno 2\] No such file or directory: 'missing/deeper\.txt'"\} but the new code produces \{'nodes': \[\], 'edges': \[\]\} (outputs first differ at character 30). Paste that input straight into a regression test.


Graphify review — findings

Fixes .svelte extraction so functions, classes and runes inside <script> are graphed instead of every component reporting a line-1 syntax error and contributing only imports. extract_svelte now parses only the script bodies via _svelte_mask_non_script, which blanks markup, style and non-JS type scripts while preserving line offsets. It picks the TS grammar unless the first block declares lang="js"/jsx, and keeps the regex import rescue as a fallback whose duplicate edges build.dedupe_edges collapses.

Worth a look

  • One-line Svelte script blocks can be swallowed by preceding line comment — graphify/extract.py:2398 · Escalate · medium
    • agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 2221 functions depend on the 286 functions this change touches.

Health — this change adds coupling hotspots:

  • new: extract() — 723 callers, 48 callees
  • new: _rebuild_code() — 147 callers, 56 callees
  • new: extract_js() — 87 callers, 4 callees
  • new: extract_xaml() — 19 callers, 17 callees
  • new: main() — 98 callers, 3 callees
  • new: dispatch_command() — 2 callers, 126 callees
  • new: _get_extractor() — 26 callers, 6 callees
  • new: run_pipeline() — 8 callers, 13 callees
  • …and 36 more — each is listed as a finding

Verification — 2221 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: 2041 function(s) in the blast radius were not formally verified this run

Test selection

Test selection

132 of 313 test file(s) selected (42%) via static blast radius.

  • tests/test_astro_extraction.py — impact
  • tests/test_astro_import_ids.py — impact
  • tests/test_blade_extractor.py — impact
  • tests/test_build.py — impact
  • tests/test_builtin_global_type_refs.py — impact
  • tests/test_case_sensitive_resolution.py — impact
  • tests/test_cjs_module_extension.py — impact
  • tests/test_cobol_extractor.py — impact
  • tests/test_cpp_nested_and_cli.py — impact
  • tests/test_cpp_objc_cross_file_calls.py — impact
  • tests/test_cross_extension_reexport_self_cycle.py — impact
  • tests/test_cross_language_call_resolution.py — impact
  • tests/test_cross_repo_external_call_guards.py — impact
  • tests/test_cross_repo_member_calls.py — impact
  • tests/test_csharp_call_site_generic_args.py — impact
  • tests/test_csharp_enum_members.py — impact
  • tests/test_csharp_field_generic_args.py — impact
  • tests/test_csharp_generic_callsites.py — impact
  • tests/test_csharp_interface_dispatch.py — impact
  • tests/test_csharp_member_calls.py — impact
  • tests/test_csharp_member_nodes.py — impact
  • tests/test_csharp_object_creation.py — impact
  • tests/test_csharp_partial_classes.py — impact
  • tests/test_csharp_tuple_type_refs.py — impact
  • tests/test_csharp_type_resolution.py — impact
  • tests/test_definition_file_portability.py — impact
  • tests/test_detect.py — impact
  • tests/test_dotnet.py — impact
  • tests/test_duplicate_annotation_edges.py — impact
  • tests/test_elixir_import_resolution.py — impact
  • tests/test_erlang_extractor.py — impact
  • tests/test_extract.py — impact
  • tests/test_extract_cache_location.py — impact
  • tests/test_extract_php_closures.py — impact
  • tests/test_file_label_disambiguation.py — impact
  • tests/test_file_node_id_spec.py — impact
  • tests/test_forwarding_review_findings.py — impact
  • tests/test_go_builtin_call_targets.py — impact
  • tests/test_go_import_repoint.py — impact
  • tests/test_go_interface_methods.py — impact
  • tests/test_go_qualified_resolution.py — impact
  • tests/test_import_extension_resolution.py — impact
  • tests/test_import_self_loops.py — impact
  • tests/test_imported_export_forwarding.py — impact
  • tests/test_incremental.py — impact
  • tests/test_indirect_call_arrow_single_param_shadow.py — impact
  • tests/test_indirect_call_block_scoped_shadow.py — impact
  • tests/test_indirect_call_catch_binding_shadow.py — impact
  • tests/test_indirect_call_external_import_shadow.py — impact
  • tests/test_indirect_call_for_of_binding_shadow.py — impact
  • … and 82 more

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.

Docs that may be stale (advisory)

…and 10 more.

Formal verification

Behavior changes: extract\_svelte changes behavior, here is the input that shows it.

The verifier found a concrete input on which extract\_svelte behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.

Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.

Evidence: On input \{"path":"\_\_import\_\_\('pathlib'\)\.Path\('missing/deeper\.txt'\)"\}, the old code produced \{'nodes': \[\], 'edges': \[\], 'error': "\[Errno 2\] No such file or directory: 'missing/deeper\.txt'"\} but the new code produces \{'nodes': \[\], 'edges': \[\]\} (outputs first differ at character 30). Paste that input straight into a regression test.

· 1 grounded finding(s) anchored inline below; 43 more finding(s) on lines outside this diff (see the check run).

Comment thread graphify/extract.py
return "".join(chars), lang


def extract_svelte(path: Path) -> dict:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Health regression — extract_svelte()

fans out to 7 callees (efferent coupling); 11 callers depend on it (afferent coupling).

Grounded coupling-delta finding (deterministic), not an LLM guess.

… read errors

Review follow-ups on Graphify-Labs#3928:

- A `//` comment closing one `<script>` block swallowed a second block on
  the same line, since masking leaves them on one line. Terminate each
  block with U+2028 then `;`: the line terminator ends the comment and
  the `;` ends a statement that had none. Line numbers and byte offsets
  are unchanged.
- An unreadable file returns the `error` key again instead of an empty
  result, matching the behavior before the masking change.
@Agnik47
Agnik47 force-pushed the fix/3928-svelte-script-masking branch from 1f37c6e to 5de47e3 Compare October 3, 2026 07:51
@safishamsi

Copy link
Copy Markdown
Member

Shipped in v0.9.76 (live on PyPI as graphifyy==0.9.76). Landed on v8 via an authorship-preserving cherry-pick, so your original commit authorship is kept. Thanks @Agnik47 for parsing only Svelte script blocks 🙏

Closing as shipped.

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

Labels

None yet

Projects

None yet

2 participants