Skip to content

fix(go): credit calls in a package-level var initialiser to the file (#2360) - #4867

Closed
nothariharan wants to merge 1 commit into
Graphify-Labs:v8from
nothariharan:fix/go-package-var-closure-calls
Closed

nothariharan wants to merge 1 commit into
Graphify-Labs:v8from
nothariharan:fix/go-package-var-closure-calls

Conversation

@nothariharan

@nothariharan nothariharan commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

What does this PR do?

Refs #2360. On v8 the Go extractor walks only function and method bodies for calls. A call made in a package-level var initialiser is never walked, so it produces no edge and the callee loses that caller. The most common form is a closure kept in a package variable as a swappable hook:

// cmd/migrate.go
package cmd

import "example.com/m/dbimpl"

var resourceDBProvider = func(name string) string {
	return dbimpl.ProvideResourceDB(name) // no edge to ProvideResourceDB
}

The same happens for a plain initialiser call such as var registry = newRegistry().

Why #2360 is not resolved yet on v8

#2360 is about package-level var/const declarations missing from the graph. On v8, extract_go's walk dispatches on function_declaration, method_declaration, type_declaration and import_declaration only. A var_declaration is recursed into and never registered as a body, so walk_calls never sees the calls in it. The two open PRs for the issue (#2364, #3098) add nodes for the declared names, but neither walks the initialiser for calls: #2364's var_declaration branch mints the nodes and returns. So the calls stay missing with either of them.

On Grafana, pkg/cmd/grafana-cli/commands/resource_db_migrate.go:22-23 holds exactly this shape:

var resourceDBProvider = func(cfg *setting.Cfg) (db.DBProvider, error) {
	return dbimpl.ProvideResourceDB(nil, cfg, nil)
}

Extracting pkg/api, pkg/api/routing, pkg/cmd/grafana-cli/commands and pkg/storage/unified/sql/db/dbimpl (74 non-test files):

  • v8: dbimpl.ProvideResourceDB (dbimpl.go:41) has 0 incoming calls edges, and resource_db_migrate.go has no edge at lines 22-23.
  • With this change: 426 → 430 calls edges, none removed. All 4 new edges are package-init calls that are in the source:
file line call
resource_db_migrate.go 23 dbimpl.ProvideResourceDB(nil, cfg, nil) inside var resourceDBProvider = func(...)
commands.go 95 runPluginCommand(installCommand) in var pluginCommands = []*cli.Command{...}
commands.go 130 runRunnerCommand(resetPasswordCommand) in var adminCommands
commands.go 151 runDbCommand(datamigrations.EncryptDatasourcePasswords) in var adminCommands

The change

extractors/go.py: a var_declaration whose parent is the source_file (a package-level one) is registered as a body owned by the file node and is walked by the existing walk_calls. That includes the bodies of closures in it, since walk_calls stops only at function and method declarations. The file is the owner because the initialiser runs at package init and has no enclosing function. That matches how other extractors handle module-level code: the JS scan of #3408, for example, attaches what a module-level closure defines to the file node. A var declared inside a function is not affected: it is still walked as part of that function.

cache.py: _AST_CACHE_SCHEMA 29 → 30, so cached Go extractions re-run.

Type of change

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

Verification & Invariants

Invariant: every call site in a Go file is attributed to the innermost named owner that runs it: a function, a method, or the file for package-level code. No call is credited to a function that does not contain it.

  • test_a_var_declared_inside_a_function_stays_with_the_function: a local var x = helper() keeps Run -> helper and adds no file-level edge.

  • Resolution is unchanged: the new raw calls go through the same in-file label match and cross-file import-evidence pass as any other Go call.

  • 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.

Not covered (unchanged):

How was this tested?

Run on Windows 11, Python 3.12, in the environment uv run --all-extras syncs (.venv; the later runs call .venv/Scripts/python -m pytest directly because uv run rewrites uv.lock on Windows).

uv run --all-extras pytest tests/test_go_package_var_calls.py -q
  # 3 passed (on unchanged v8: 2 failed, 1 passed; the local-var negative passes on both)
uv run --all-extras pytest tests/test_go_*.py tests/test_sql_go_batches.py tests/test_languages.py tests/test_cache*.py -q
  # 696 passed
uv run --all-extras pytest tests -q
  # 8877 passed, 10 failed, 99 skipped. All 10 are in files that fail on unchanged v8 on Windows\n  # (test_code_only_update_all_hosts x2, test_hooks, test_language_override_lifecycle,\n  # test_resolve_memoization, test_source_key_memoization, test_unmapped_at_alias_resolution).\n  # test_hooks.py run alone gives 1 failure (test_shebang_parse_requires_leading_hash_bang), the same on v8.
uv run ruff check graphify/extractors/go.py graphify/cache.py tests/test_go_package_var_calls.py
  # All checks passed!

Graphify-specific checklist

  • I updated generated skill artifacts (uv run python -m tools.skillgen --bless) when changing their source fragments.
  • I confirmed that AST/structural extraction remains deterministic (no ambient state dependencies like ENV variables).
  • 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.

…raphify-Labs#2360)

extract_go walks only function and method bodies for calls. A package-level
`var` initialiser was recursed into and never registered as a body, so a
call there, most often inside a `var provider = func(...) {...}` closure
kept as a swappable hook, produced no edge and its callee lost the caller.

A package-level var_declaration is now walked by the existing walk_calls,
owned by the file node: the initialiser runs at package init and has no
enclosing function. A `var` inside a function is unchanged. The AST cache
schema goes to 30.
@github-actions

Copy link
Copy Markdown

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

@github-actions github-actions Bot added the contributor Pull request from a returning community contributor label Oct 30, 2026

@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.

No issues found by static checks — no coupling regressions or blocking issues in the code graph. That is not the same as safe to merge: see what was not checked.

Not checked: tests were not run; formal verification proved 0 of 1 changed function(s) (1 sampled, not proven).

Formal verification. PR-changed functions: 1/1 verified (0 proven, 1 may-equivalent, 0 distinguished) · 0 not verified.


Graphify review — findings

Walks calls inside Go package-level var initialisers, including those inside a var f = func() {...} closure, and credits them to the file node (#2360). Previously these calls were skipped, so edges from package-init code never appeared in the graph. Bumps _AST_CACHE_SCHEMA to 30 so cached Go extractions re-run and pick up the new edges.

No blocking issues surfaced. 1 lower-confidence candidate did not survive cross-model review.

Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 2861 functions depend on the 125 functions this change touches.

Health — this change adds coupling hotspots:

  • worse: extract() — 1055 callers, 62 callees

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

Test selection

Test selection

51 of 470 test file(s) selected (11%) via static blast radius.

  • tests/test_binary_media_words.py — impact
  • tests/test_cache.py — impact
  • tests/test_cache_long_paths.py — impact
  • tests/test_cache_stale_import_target.py — impact
  • tests/test_charmap_encoding.py — impact
  • tests/test_chunking.py — impact
  • tests/test_csharp_partial_classes.py — impact
  • tests/test_definition_file_portability.py — impact
  • tests/test_detect.py — impact
  • tests/test_extract.py — impact
  • tests/test_extract_cache_location.py — impact
  • tests/test_extract_cli.py — impact
  • tests/test_go_package_var_calls.py — impact, changed-test
  • tests/test_go_type_alias.py — impact
  • tests/test_grammar_cache_key.py — impact
  • tests/test_ignore_file_encoding.py — impact
  • tests/test_incremental_mtime_collision.py — impact
  • tests/test_java_member_calls.py — impact
  • tests/test_js_member_collisions.py — impact
  • tests/test_language_override_lifecycle.py — impact
  • tests/test_language_overrides.py — impact
  • tests/test_languages.py — impact
  • tests/test_llm_backends.py — impact
  • tests/test_manifest_ingest.py — impact
  • tests/test_msbuild_dir_evidence.py — impact
  • tests/test_multilang.py — impact
  • tests/test_objc_field_table_remap.py — impact
  • tests/test_out_dir_evidence.py — impact
  • tests/test_partial_cache.py — impact
  • tests/test_pipeline.py — impact
  • tests/test_python_inherited_method_calls.py — impact
  • tests/test_python_injected_factory_calls.py — impact
  • tests/test_python_property_reads.py — impact
  • tests/test_python_pytest_fixture_receivers.py — impact
  • tests/test_rust_self_member_calls.py — impact
  • tests/test_rust_use_trees.py — impact
  • tests/test_semantic_cache_basename_recovery.py — impact
  • tests/test_semantic_cache_out_root.py — impact
  • tests/test_skill_persisted_scan_options.py — impact
  • tests/test_skill_semantic_all_cached.py — impact
  • tests/test_skillgen.py — impact
  • tests/test_stale_prune.py — impact
  • tests/test_stat_index_husk.py — impact
  • tests/test_stat_index_multi_root.py — impact
  • tests/test_stat_index_portability.py — impact
  • tests/test_update_no_change_gate.py — impact
  • tests/test_vbnet_extractor.py — impact
  • tests/test_watch.py — impact
  • tests/test_watch_manifest_location.py — impact
  • tests/test_word_count_cache.py — impact
  • … and 1 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.

Formal verification

No difference found (not proven): No behavior difference found in extract\_go (not a proof).

The verifier ran both versions of extract\_go on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.

Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.

Note: An input the sampler did not try could still differ.

· 1 more finding(s) on lines outside this diff (see the check run).

safishamsi added a commit that referenced this pull request Oct 30, 2026
Go (#4867/#4868/#4866) and Python (#4773) extractor changes alter
per-file AST output and are AST-cached, so one combined _AST_CACHE_SCHEMA
bump 30->31 forces stale caches to re-extract. Install (#4770/#4771) and
the skill reword (#4721) need no bump. Folded into the unreleased 0.9.99.
Each PR omitted its own bump / carried a stale number; the single combined
bump lands here.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@safishamsi

Copy link
Copy Markdown
Member

Landed in v0.9.99, now on PyPI (pip install graphifyy==0.9.99). A call in a package-level var initialiser (and its closures) is credited to the file node. Thanks for the fix, @nothariharan.

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

Labels

contributor Pull request from a returning community contributor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants