Repository navigation
Conversation
|
Thanks for the pull request, @krishhgg. 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[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 1 advisory finding(s) below merit a look before merge.
Formal verification. PR-changed functions: 1/2 verified (0 proven, 1 may-equivalent, 0 distinguished) · 1 not verified (1 vacuous).
Not verified on this run: extract (vacuous: never exercised).
Graphify review — findings
Adds tsconfig customConditions support to JS/TS workspace import resolution, so a project that sets e.g. "@zod/source" resolves a workspace package through that condition's exports entry (./src/index.ts) instead of build-only import/require targets. _read_tsconfig_exports_options merges the relevant compilerOptions across relative extends chains the way tsc does, and _resolve_custom_condition_export handles only the shape where the custom condition is the first key and points at a TypeScript implementation file. Whenever tsc's outcome isn't modelled, the custom-condition lookup is skipped and the existing condition walk applies: unparseable or symlinked configs, package-name extends, a non-exports moduleResolution, matching paths, any baseUrl/outDir/declarationDir, or non-empty moduleSuffixes.
Worth a look
- Escaped JSON customConditions are discarded —
graphify/extractors/resolution.py:359· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Review partial — this diff was larger than one review pass covers, so later files were not reviewed; some findings may be missing.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 2771 functions depend on the 576 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 778 callers, 50 callees - new:
_rebuild_code()— 149 callers, 56 callees - new:
_extract_generic()— 18 callers, 31 callees - new:
_resolve_js_module_path()— 42 callers, 9 callees - new:
extract_js()— 87 callers, 4 callees - new:
extract_xaml()— 19 callers, 17 callees - new:
main()— 102 callers, 3 callees - new:
dispatch_command()— 2 callers, 128 callees - …and 59 more — each is listed as a finding
Verification — 2771 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: 2581 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
151 of 334 test file(s) selected (45%) via static blast radius.
tests/test_astro_extraction.py— impacttests/test_astro_import_ids.py— impacttests/test_blade_extractor.py— impacttests/test_build.py— impacttests/test_builtin_global_type_refs.py— impacttests/test_cache.py— impacttests/test_case_sensitive_resolution.py— impacttests/test_cjs_module_extension.py— impacttests/test_cobol_extractor.py— impacttests/test_cpp_method_declarations.py— impacttests/test_cpp_nested_and_cli.py— impacttests/test_cpp_objc_cross_file_calls.py— impacttests/test_cross_extension_reexport_self_cycle.py— impacttests/test_cross_language_call_resolution.py— impacttests/test_cross_repo_external_call_guards.py— impacttests/test_cross_repo_member_calls.py— impacttests/test_csharp_call_site_generic_args.py— impacttests/test_csharp_enum_members.py— impacttests/test_csharp_field_generic_args.py— impacttests/test_csharp_generic_callsites.py— impacttests/test_csharp_interface_dispatch.py— impacttests/test_csharp_member_calls.py— impacttests/test_csharp_member_nodes.py— impacttests/test_csharp_object_creation.py— impacttests/test_csharp_partial_classes.py— impacttests/test_csharp_tuple_type_refs.py— impacttests/test_csharp_type_resolution.py— impacttests/test_definition_file_portability.py— impacttests/test_detect.py— impacttests/test_dotnet.py— impacttests/test_duplicate_annotation_edges.py— impacttests/test_elixir_import_resolution.py— impacttests/test_elixir_unqualified_call_scope.py— impacttests/test_erlang_extractor.py— impacttests/test_extract.py— impacttests/test_extract_cache_location.py— impacttests/test_extract_php_closures.py— impacttests/test_file_label_disambiguation.py— impacttests/test_file_node_id_spec.py— impacttests/test_forwarding_review_findings.py— impacttests/test_go_builtin_call_targets.py— impacttests/test_go_import_repoint.py— impacttests/test_go_interface_methods.py— impacttests/test_go_qualified_resolution.py— impacttests/test_import_extension_resolution.py— impacttests/test_import_self_loops.py— impacttests/test_imported_export_forwarding.py— impacttests/test_incremental.py— impacttests/test_indirect_call_arrow_single_param_shadow.py— impacttests/test_indirect_call_block_scoped_shadow.py— impact- … and 101 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)
BENCHMARKS.md§ graphify Benchmarks (lines 1-8): references changed symbolsconditionsCHANGELOG.md§ 0.9.17 (2026-07-16) (lines 754-783): references changed symbolsconfigCHANGELOG.md§ 0.9.6 (2026-07-04) (lines 950-975): references changed symbolsconfigCHANGELOG.md§ 0.8.28 (2026-06-01) (lines 1308-1316): references changed symbolsconfigCHANGELOG.md§ 0.8.25 (2026-05-29) (lines 1336-1345): references changed symbolsconfigCHANGELOG.md§ 0.8.21 (2026-05-27) (lines 1364-1377): references changed symbolsconfigCHANGELOG.md§ 0.7.13 (2026-05-09) (lines 1594-1600): references changed symbolsconfigCHANGELOG.md§ 0.7.0 (2026-05-03) (lines 1708-1718): references changed symbolsconfigCHANGELOG.md§ 0.4.19 (2026-04-17) (lines 1881-1890): references changed symbolsconfigCHANGELOG.md§ 0.4.10 (2026-04-13) (lines 1938-1957): references changed symbolsconfig
…and 10 more.
Formal verification
Could not verify: Could not verify extract.
The verifier did not have enough to check extract, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: no capturable inputs from the test suite; property tier: not verifiable: all 90 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly TypeError — names the real obstacle, not a sampling gap)
No difference found (not proven): No behavior difference found in \_resolve\_workspace\_import (not a proof).
The verifier ran both versions of \_resolve\_workspace\_import 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.
· 3 grounded finding(s) anchored inline below; 64 more finding(s) on lines outside this diff (see the check run).
| return True | ||
| return False | ||
|
|
||
| def _load_tsconfig_custom_conditions(start_dir: Path, specifier: str) -> tuple[str, ...]: |
There was a problem hiding this comment.
_load_tsconfig_custom_conditions()
high coupling complexity (Ca·Ce = 20).
Grounded coupling-delta finding (deterministic), not an LLM guess.
| return None | ||
| return found | ||
|
|
||
| def _resolve_workspace_import(raw: str, start_dir: Path) -> Path | None: |
There was a problem hiding this comment.
_resolve_workspace_import()
fans out to 7 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
| "case", | ||
| ["empty-outdir", "empty-declarationdir", "pattern-precedence", "multi-star", "jsonc-condition"], | ||
| ) | ||
| def test_tsconfig_custom_condition_keeps_v8_for_output_dirs_patterns_and_jsonc( |
There was a problem hiding this comment.
test_tsconfig_custom_condition_keeps_v8_for_output_dirs_patterns_and_jsonc()
fans out to 7 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
|
On the advisory about escaped The coupling notes are on |
Workspace imports resolve exports through a fixed condition list that ignores the importer's customConditions. Packages such as zod expose TypeScript sources under a custom key beside runtime targets that exist only after a build. Keep the existing walk, including v8's dist-to-src fallback (Graphify-Labs#4485). Only when it finds no source file, try the first custom-condition key of an exact exports entry. Require an in-package TypeScript implementation that tsc reaches through its extension lookups. Refuse uncertain config, path and exports shapes, and keep v8's result in those cases. Merge the importer's tsconfig extends chain with a per-extraction cache. Cover source-twin precedence for built and unbuilt workspace packages, per-importer options, cache refresh and conservative refusal boundaries. Refuse JSONC configs whose string contents would change during stripping, including parent paths, so unrelated literals cannot validate a corrupt value. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Co-Authored-By: Codex <noreply@openai.com>
4cb7f73 to
c1e5dcb
Compare
This branch has not been deployed
1 participant
What does this PR do?
Workspace package imports ignore the importer's
compilerOptions.customConditions. For example, zod puts its checked-in TypeScript source behind"@zod/source", alongside runtime exports that exist only after a build. On current v8, 229 distinct zod file/specifier sites still resolve to no file.This adds a conservative fallback after the existing workspace resolution walk. It preserves every source file that walk reaches, including the dist-to-src fallback from #4485. Only an import left unresolved or on build output can change.
The fallback reads the importer's nearest tsconfig.json or jsconfig.json, merges relative or absolute
extendsparents in TypeScript's order, and caches completed merges per extraction run. It handles an exact exports entry whose first condition key is active in that importer and whose value names an in-package TypeScript implementation. Extension substitutions follow TypeScript 5.9.3. Declarations and JavaScript-only targets are refused.It keeps v8's result when the config or target is uncertain:
resolvePackageJsonExports: false.baseUrl,outDir, ordeclarationDir; nonempty module suffixes; matchingpathskeys; or path-option templates./.Numeric-key rejection uses string checks, so an arbitrarily long key cannot raise an integer-conversion exception.
Verification against v0.9.92
Base:
86c57be8204673c2a1b9849a3292a7dd3ab373d8.Candidate:
c1e5dcb9e8d32ae966d5757f05315a5214ad0d4b.The diff adds 1,192 lines across three files. It preserves both sets of tests at the rebase conflict and adds cases for source-twin priority, JSONC strings, numeric-key limits, and lexical importer paths.
graphify update . --no-clustercompletes.Fresh CLI extractions used
extract --code-only --no-cluster --no-dedup --max-workers 1with separate output directories for baseline and candidate. The TypeScript 5.9.3 oracle uses each importer's own config and module resolution mode.These scores cover immediate file import pairs within mapped oracle files. They do not measure symbol or call precision. Zod maps 891 of 896 oracle pairs; hono maps 843 of 860; express maps all 153. The existing 12 zod and 3 hono false-positive pairs remain unchanged.
All 229 changed zod file/specifier sites were unresolved on v8 and match TypeScript's selected file on this branch. Zod keeps all 5,922 existing nodes with all attributes unchanged, and all existing edges except 203 imports_from edges to the external
ref_zodstub. In particular, all 1,903 existing imports to source files retain every attribute. Its graph changes from 10,004 to 10,090 edges and adds six external symbol stubs. Hono and express graph.json files are byte-identical.Limits
This deliberately supports fewer shapes than TypeScript. Existing v8 source targets always win, even where its fixed condition priority or source-twin heuristic differs from TypeScript. Solution-style
references, package.jsonimportsconditions, JavaScript-only custom targets, and the refused shapes above remain outside this fallback.Importer edits followed by changed-paths rebuilds match full CLI rebuilds in independent fixtures. Config-only edits can stay stale until a full
graphify update, matching the existing alias-loader limitation. Windows was not tested; symlink tests skip where symlinks are unavailable.The full suites ran with the development dependency group and an isolated SQL grammar. Other optional-dependency tests can skip. No generated skill sources changed. Both material AI contributors are disclosed in the commit metadata.