Resync with graphify-py @ 544f95e (v0.9.2)#33
Merged
Conversation
Port the v0.9.0..v0.9.2 changes from the `graphify-py` reference into the Rust workspace. Extraction (`graphify-extract`): - tsconfig `paths` keep all targets, trying each on disk (#1531) - package.json `exports` subpath map with condition priority, a single `./*` wildcard, and a path-escape guard (#1308) - alias/workspace import targets remap via the absolute-resolved id form so relative-input runs do not orphan symbol edges (#1529) - skip Java type-parameter references; emit record-component references (#1518, #1519) - type-aware Ruby member-call resolution: `Class.new` and typed `var.method` resolve by receiver type, dispatched through a small resolver registry (#1499) - Swift type-qualified static calls are EXTRACTED, locally-inferred instance calls stay INFERRED (#1533) - ObjC: blank `NS_ASSUME_NONNULL` macros, resolve a quoted `#import` to the real header, emit `[[Foo alloc] init]` type references (#1475) - salt separator-colliding node ids with a path hash; repoint ObjC header imports to the disambiguated `.h` node (#1522, #1475) - record `origin_file` on the Go reference stub; drop `origin_file` from extract output so no absolute path leaks (#1515, #1516) Other crates: - `prune_semantic_cache` sweeps orphaned content-hash entries, wired into the extract command (#1527) - GraphML export pins null-attribute coercion to the empty string (#1502) - `GRAPHIFY_MAX_RETRIES` plus a 429 retry loop around the LLM HTTP sends; the secondary dispatch path keeps its timeout (#1523, #1442) - `graphify update` prunes edges owned by a re-extracted file (#1521) - skill and AGENTS install strings drop the host-specific `skill` tool reference (#1530) - `save-result --answer-file` reads the answer from a file (#1502) Bump the workspace version to 0.9.2 in lockstep with the submodule pointer and document `--answer-file` and the retry env vars in `USAGE.md`. Glory to the Omnissiah
Apply the actionable findings from the first CodeRabbit pass. - `contained_in_package` canonicalises the nearest existing ancestor of the candidate so a symlink that crosses the package boundary is rejected, not just a lexical `..` escape (closer to Python's `Path.resolve()`). - correct the `send_json_with_retry` doc: only HTTP 429 is retried, 5xx fails fast. - assert the exact default retry count (6) instead of a lower bound. - mark the env-mutating 429 retry tests `#[serial(env)]`. - route aliased imports through `resolve_js_module_path` in the Svelte and JS import branches so a `.js` specifier backed by a `.ts` source hashes to the same node id; add a regression test. - select the file node by label in the non-salted id test so it is order-independent. - `USAGE.md`: `--answer-file` takes precedence when both answer inputs are given; at least one is required. By the will of the Machine God
CodeRabbit flagged three latent bugs in the colliding-node-id pass. The `graphify-py` reference has the same gaps, but per AGENTS.md a bug in the reference is not a requirement, and each violates an observable contract in a collision corner case. The divergences are documented in code. - restrict the header-variant repoint to C-family importers. A non-C `imports_from` whose target merely collides with a C header id was silently rewritten to the header, mis-targeting the edge. - only repoint to a header when the colliding group holds exactly one header candidate; the reference picks the first in node order, which is arbitrary when two same-stem headers (`foo.h`/`foo.hpp`) collide. - guard a salted id against colliding with an id surviving outside its group, widening the salt until globally unique. `occupied` holds the surviving ids (non-ambiguous, plus ambiguous groups with an empty-key member that keeps the bare id), so the guard never over-hashes against an id that is itself about to be rewritten. Extract `rewrite_edge_endpoints` to keep the pass under the line limit; add regression tests for the non-C import and the cross-group salt. By the will of the Machine God
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR bumps the workspace to version 0.9.2 and adds Ruby type-aware cross-file call resolution, tsconfig alias and workspace export resolution updates, node-ID collision handling changes, ObjC and Java extractor changes, LLM retry handling, semantic cache pruning, watch edge eviction, and a ChangesExtraction pipeline enhancements
LLM retry
Cache, watch, CLI, hooks, and versioning
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Resolve the second review round; divergences from `graphify-py` are documented in code comments (where the reviewer reads them). Resolution accuracy: - Ruby: index class-definition nodes from `contains` (not only `method` edges) so a method-less class still resolves `Class.new`; defer Ruby member calls before the same-file bare-name lookup so a typed `p.run` resolves by receiver type instead of being swallowed by a local `run`; skip nested `class`/`module` scopes in the local-binding walk. - Swift: a receiver bound as a local resolves via its inferred type and is INFERRED even when upper-cased, so an upper-cased local is no longer mis-marked as a type-qualified EXTRACTED call. Id disambiguation and resolution: - postprocess: track ids minted within a pass so a later collision group can't reuse an earlier group's salted id, while still collapsing same-file same-id nodes onto one remap entry. - tsconfig: match the longest alias prefix so `@/feature/*` wins over a broad `@/*`. - workspace: resolve a bare conditional-`exports` object (no `.` key) as the package entry, and reject `exports` targets that escape the package directory on the no-subpath path too. CLI / cache: - `--answer-file` content is preserved exactly (no trim), matching inline `--answer`. - the semantic-cache prune resolves relative paths against `root`, as the cache lookup does, so a valid entry isn't pruned on relative input. Tests and docs: - cover the negative `GRAPHIFY_MAX_RETRIES` fallback and assert the no-retry mock is hit exactly once; assert the full save-result requirement message; document `GRAPHIFY_RETRY_BASE_MS`. - regression tests for the method-less Ruby class and the cross-group salt collision; `#[must_use]` on `default_resolvers`; new tests use `?`-returning signatures. By the will of the Machine God
CodeRabbit asked the test to assert the non-C import resolves to the Python variant, but the salt remap is keyed by the importer's own file and cannot infer the target module for a bare ambiguous import when the importer differs from the definition (the graphify-py reference doesn't either). Document that in the test and assert the gate the fix actually guarantees -- a non-C import is never repointed at the header variant -- rather than pinning a dangling id. By the will of the Machine God
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
The file-wide local type table is not scope-aware, so consulting it before the receiver's syntax could demote a genuine `Type.staticMethod()` to an INFERRED call on whatever type a same-named local was bound to elsewhere in the file. Resolve an upper-cased receiver as the named type (EXTRACTED) and consult the table only for lower-cased receivers, which re-aligns with graphify-py (extract.py:9553) and removes the divergence. Add a regression test for the pollution case (an upper-cased parameter binding alongside a real type-qualified call). The companion review note about `member_call_captures_receiver` missing `#[test]` is a false positive: the attribute is present and the test runs (confirmed via `nextest -E 'test(member_call_captures_receiver)'`). By the will of the Machine God
rblaine95
force-pushed
the
resync/graphify-py/544f95e
branch
from
5620bfc to
ec1c325
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ports
graphify-pyv0.9.0→v0.9.2(commit544f95e) into the Rust workspace, advancing the submodule pointer and bumping the workspace version in lockstep. The submodule update is its own commit (0d4f019); the Rust ports follow.Ported
graphify-extractpathstargets and resolve each on disk in order.exportssubpath map (#1308): resolveexportsconditions (prioritysource > import > module > svelte > types > require > default) with a symlink-safe package-containment guard.var = ClassName.newbindings and resolvevar.method()by receiver type.Type.staticMethod()isEXTRACTED(1.0); instance calls inferred via local type areINFERRED(0.8).NS_ASSUME_NONNULLmacros before parsing; resolve quoted#importto real files; emitalloc/initreferences.origin_filehandling (#1515/#1516): droporigin_filefrom the final output (internal disambiguation hint); set it on Go cross-file stubs so same-label stubs split per file.Other crates
graphify-cache:prune_semantic_cache+ CLI wiring (#1527) — sweep orphaned semantic-cache entries after extraction.graphify-export: GraphML null-attribute coercion parity test (#1502).graphify-llm:GRAPHIFY_MAX_RETRIES(default 6) + a 429/transient retry loop onureqsends with exponential backoff (#1523/#1442).graphify-watch: prune edges owned by a re-extracted file (#1521).graphify-hooks: skill install-string update (#1530).save-result --answer-file(#1502).Docs: workspace version →
0.9.2(EXTRACTOR_VERSIONbumped, invalidating the old AST cache);GRAPHIFY_MAX_RETRIES,GRAPHIFY_API_TIMEOUT,GRAPHIFY_MAX_OUTPUT_TOKENS, and--answer-filedocumented inUSAGE.md.Verification
cargo nextest run --workspace: 2333 passed (2 feature-gated skips).cargo clippy --all-targets --all-features --workspace: clean.hk check: clean.graphify-py/tests/test_*.pycase.CodeRabbit
Three review rounds. Round 1's 7 actionable findings fixed; round 2 surfaced three latent bugs in the colliding-node-id pass (header remap mis-targeting non-C imports, ambiguous header selection, and a salt that could collide outside its group) — these matched the
graphify-pyreference, but perAGENTS.mda bug in the reference is not a requirement, so they're fixed in Rust with the divergence documented in code comments and covered by regression tests. Final review: 0 findings.Divergences from
graphify-pycrates/graphify-extract/src/postprocess.rscomments).GRAPHIFY_RETRY_BASE_MSis a Rust-only test seam for the retry backoff base.Coverage note
The
≥95%-per-touched-crate target is a pre-existing baseline gap (the workspace sits ~85%). New code is well-covered (resolver framework 100%, Ruby resolver ~95%, GraphML null path 100%); the shortfall is untouched pre-existing low-coverage files, not a regression from this resync.Summary by CodeRabbit
save-resultnow supports--answer-file, with--answer-filetaking priority over--answer.GRAPHIFY_MAX_RETRIES,GRAPHIFY_RETRY_BASE_MS,GRAPHIFY_API_TIMEOUT).nullattribute values.0.9.2, and expanded parity/integration test coverage.