Phase1: tree sitter dependency extraction for diffgraph - #13
Conversation
- Add TreeSitterProcessor with multi-language support framework - Implement Python component extraction (classes, functions, methods) - Use tree-sitter-language-pack for AST parsing - Support Python, TypeScript, JavaScript, Go, Rust, Java, Swift (Python fully working) - Extract component relationships from static analysis - Register as 'tree-sitter-dependency-graph' mode - Add basic test demonstrating Python extraction Component extraction includes: - Classes/containers with nested methods - Standalone functions - Import statements - Function call analysis for dependency mapping Successfully tested with Python code showing proper extraction of: - 1 class (MyClass) - 2 methods (__init__, increment) - 2 functions (standalone_function, another_function) Next steps: - Complete TypeScript/JavaScript extraction (partial implementation) - Complete Go, Rust, Java, Swift extraction (partial implementation) - Add comprehensive testing across all supported languages - Enhance dependency detection for cross-file references
- Add tree-sitter-dependency-graph to available modes in README - Document Python support as fully working - Note other languages as in progress - Update CHANGELOG with phase 1 implementation details - Add usage examples for tree-sitter mode
- Document completed work: Python extraction fully functional - Detail in-progress work for other languages - Explain tree-sitter API migration (0.25+) - Provide technical insights and lessons learned - Outline next steps for phases 2-4 - Include usage examples and performance notes
Move documentation files to docs/: - PHASE1_SUMMARY.md -> docs/ - GRAPH_EXPORT_FEATURE.md -> docs/ - TESTING_GUIDE.md -> docs/ Move test files to tests/: - test_tree_sitter_basic.py -> tests/ - test_graph_export.py -> tests/ - test_structured_export.py -> tests/ - test_cli_manual.sh -> tests/ Move example to tests/examples/: - example_usage.py -> tests/examples/ This improves project organization and follows standard conventions.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughThe PR adds a tree-sitter processing mode with multi-language AST extraction, schema v2 output, deterministic relationships, optional processor loading, graph export validation, CI coverage, and usage documentation. ChangesTree-sitter analysis and structured export
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant TreeSitterProcessor
participant ASTParser
participant SchemaV2Adapter
participant CLI
TreeSitterProcessor->>ASTParser: Parse changed file contents
ASTParser->>TreeSitterProcessor: Return symbols and imports
TreeSitterProcessor->>SchemaV2Adapter: Provide snapshots and file changes
SchemaV2Adapter->>TreeSitterProcessor: Return schema v2 analysis
CLI->>TreeSitterProcessor: Request graph export
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
Addresses the two blocking gaps from PR-13-REVIEW.md: Gap 1 — Output format: new schema_v2_adapter.py module produces symbols[] + relationships[] with analysis_source='structural' and evidence pointers, conforming to diffgraph-v2.schema.json. Gap 2 — Symbol diff layer: compute_symbol_diff() compares pre-change and post-change AST snapshots to assign change_kind (added / modified / deleted / unchanged) to every symbol. Uses content-slice comparison so symbols that shift in line number without changing body are correctly marked 'unchanged'. Also adds: - _get_pre_change_content() / _get_post_change_content() to TreeSitterProcessor (Gap 7 — explicit pre/post split) - analyze_changes_v2() entry point on TreeSitterProcessor, which calls the adapter and returns a schema v2 dict. No GraphManager, no Mermaid, no network calls. metadata.privacy_tier='local' always. - Guard openai_agents_dependency import with try/except so the module loads without the openai-agents SDK installed. - 21 deterministic unit tests (test_schema_v2_adapter.py), including a zero-network-calls assertion (Gap 6).
…inst diffgraph-v2.schema.json
Five concrete gaps fixed:
1. _get_parser: was calling tslp.get_parser() (incompatible native API); now
uses ts.Parser(tslp.get_language()) so node.type / node.start_point /
node.children all work correctly. This was silently swallowed by except/pass,
causing zero symbols and zero imports on every run.
2. build_symbol_entry: added required schema v2 fields (name, file_id, kind,
parent_id); renamed file→file_id; flattened evidence location (was nested
under location{}, schema expects flat file/line_start/line_end on evidence).
3. build_import_relationship: renamed from/to → source_id/target_id; added id
field with format rel::<src>-><tgt> matching schema pattern requirement.
4. build_schema_v2_output: diff_ref had disallowed from/to fields (schema has
additionalProperties:false); warnings was at top-level (schema has it in
metadata.warnings); files entries were missing id and analysis_source.
5. tests: updated test_schema_v2_adapter assertions for new field names;
updated test_tree_sitter_basic to use schema v2 dict API (not old
DiffAnalysis object).
Result: 45 tests pass (up from 40); all Phase 1 acceptance criteria met:
- analyze_changes() returns schema v2 dict
- privacy_tier == local
- output validates against diffgraph-v2.schema.json
- relationships[] includes import relationships for Python
- metadata.analysis_duration_ms present
Three changes that form the privacy contract for v2: 1. BaseProcessor: replace DiffAnalysis return type with dict (schema v2), add privacy_tier abstract property, add class-level description attribute. list_available_modes() now reads description without instantiation — fixes the unsafe __new__ pattern. 2. consent.py: one-time consent prompt for cloud-tier processors. Stored in ~/.config/wild/config.json. Local-tier processors bypass it entirely. 3. cli.py: default mode flipped from openai-agents-dependency-graph to local-structural. Consent check added before analyze_changes(). 4. local_structural.py: placeholder LocalStructuralProcessor that registers the local-structural mode and delegates to TreeSitterDependencyProcessor when Phase 1 (PR #13) is present. Prints an actionable message and exits cleanly when tree-sitter is unavailable. 5. openai_agents_dependency.py: lazy import for agents SDK so wild diff (local mode) works without the SDK installed. Added privacy_tier = 'cloud_llm'. SDK-dependent tests skip gracefully. Phase 2 acceptance criteria from V2-IMPLEMENTATION-ROADMAP.md: all met.
Implements the Phase 3 gap fixes from design/PR-11-REVIEW.md. Changes: - structured_export.py: add transform_to_diffgraph_v2() + export_diffgraph_v2(). Produces symbols[]/relationships[] (schema v2) from GraphManager. Files get analysis_source='structural' (git metadata, schema const). Symbols and relationships get analysis_source='inferred' + evidence array. change_kind (not change_type), privacy_tier in metadata, schema_version at root. Classification (is_test) replaces old category string. lines_added/lines_removed replaces old stats.additions/deletions. Output validates against diffgraph-v2.schema.json when jsonschema installed. - cli.py: --format choices are now 'html' | 'json' (replaces 'html' | 'graph'). --graph-format and graph_export.py import removed. wild diff --format json writes a schema-v2 artifact. - graph_export.py: deprecated with warning; will be removed in v2.0.0. - diffgraph/schema/diffgraph-v2.schema.json: copied from main (merged via PR #16). Used for validation in export_diffgraph_v2(validate=True). - tests/test_diffgraph_v2_export.py: 35 tests, zero network calls. Covers diff_ref derivation, all FileEntry/SymbolEntry/RelationshipEntry fields, schema validation (jsonschema), file I/O, and the key Phase 3 acceptance criteria. Phase 3 acceptance criteria (from IMPLEMENTATION-STATUS.md): all met. Merge order: still depends on Phase 1 (PR #13) merging first so tree-sitter LocalStructuralProcessor can populate the schema-v2 dict natively.
nia-sg-bot
added
priority:P0
Product-direction review — 2026-08-01Verdict: REVISE · P0 implementation seed This is the strongest producer candidate, but it should not merge as the current stacked branch. Rebuild/rebase its deterministic subset on current This is a scope/alignment review, not an automatic closure decision. Items marked DISCUSS CLOSE should be resolved with maintainer context before closing. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (8)
docs/GRAPH_EXPORT_FEATURE.md-164-180 (1)
164-180: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse repository-relative script paths.
The documented commands use files as if they are in the current directory. The reviewed files are
tests/test_graph_export.pyandtests/examples/example_usage.py. These commands fail when users run them from the repository root.Proposed documentation fix
- python test_graph_export.py + python tests/test_graph_export.py ... - python example_usage.py my-changes.json + python tests/examples/example_usage.py my-changes.json🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/GRAPH_EXPORT_FEATURE.md` around lines 164 - 180, Update the Testing commands in GRAPH_EXPORT_FEATURE.md to use repository-relative paths for tests/test_graph_export.py and tests/examples/example_usage.py, while leaving the wild diff example unchanged.diffgraph/processing_modes/tree_sitter_dependency.py-746-761 (1)
746-761: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDispatch drops
tsxand mixes grammars for JavaScript.
LANGUAGE_CONFIGSdeclarestsxat lines 65-68, but_extract_componentshas notsxbranch, so.tsxfiles fall through toreturn []. The file is still counted inlanguages_detectedand reported with zero symbols.
javascriptis routed to_extract_components_typescript, which builds its queries from the TypeScript grammar at line 283 while_get_parserparses the tree with the JavaScript grammar. The node types differ; JavaScript class names areidentifier, nottype_identifier. Pass the detected language into the extractor and load the matching grammar.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@diffgraph/processing_modes/tree_sitter_dependency.py` around lines 746 - 761, The _extract_components dispatch must handle tsx and stop using TypeScript queries for JavaScript. Pass the detected language through the relevant extraction flow, route tsx appropriately, and update _extract_components_typescript or its grammar-loading logic to use the matching JavaScript, TypeScript, or TSX grammar so query node types align with the parser returned by _get_parser.CHANGELOG.md-11-16 (1)
11-16: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winClarify the implemented language status.
The entry lists TypeScript, JavaScript, Go, Rust, Java, and Swift beside Python without saying that they are incomplete. State that Python is fully supported and the other languages are framework or parser work in progress.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@CHANGELOG.md` around lines 11 - 16, Update the “Tree-sitter Dependency Extraction (Phase 1)” changelog entry to clearly distinguish Python as fully supported, while describing TypeScript, JavaScript, Go, Rust, Java, and Swift as framework/parser work in progress.docs/PHASE1_SUMMARY.md-186-190 (1)
186-190: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winReplace the developer-specific absolute path.
/Users/apple/Work/Personal/opensourceis not portable and exposes a local filesystem layout. Use a repository-relative example or an environment variable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/PHASE1_SUMMARY.md` around lines 186 - 190, Replace the hard-coded developer-specific path in the “Priority 3: Testing & Validation” section with a portable repository-relative example or an environment-variable-based location, while preserving the instruction to use real repositories for validation.docs/PHASE1_SUMMARY.md-131-136 (1)
131-136: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRemove the component that is not in the input.
The sample input defines
MyClass,__init__,increment, andstandalone_function. It does not defineanother_function. Add that function to the input or remove it from the expected output.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/PHASE1_SUMMARY.md` around lines 131 - 136, Update the extracted-components example in PHASE1_SUMMARY so the expected output matches the sample input: remove another_function from the “Output components extracted” list, unless the input is explicitly updated to define it.docs/PHASE1_SUMMARY.md-82-95 (1)
82-95: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse
ts.Parser(...)for the Tree-sitter parser example/pin.The implementation creates the parser with
ts.Parser(tslp.get_language(...)), nottslp.get_parser(language). Keep the docs and dependency declarations aligned with the API used byTreeSitterProcessor._get_parser().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/PHASE1_SUMMARY.md` around lines 82 - 95, The Tree-sitter documentation example in “Tree-sitter API (v0.25+)” uses the wrong parser construction. Update it to demonstrate the same ts.Parser-based initialization used by TreeSitterProcessor._get_parser(), and align any nearby dependency/API guidance with that implementation instead of tslp.get_parser(language).docs/PHASE1_SUMMARY.md-154-154 (1)
154-154: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove the unsupported runtime claim.
setup.pydeclarespython_requires=">=3.7"and no CI or package metadata declares Python 3.13. Keep thetree-sitter-language-packrequirement, but do not assert that the project runs on Python 3.13.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/PHASE1_SUMMARY.md` at line 154, Update the Python compatibility note in PHASE1_SUMMARY.md to remove the claim that the project runs on Python 3.13, while retaining the tree-sitter-language-pack requirement and only stating supported versions backed by setup.py or project metadata.docs/PHASE1_SUMMARY.md-145-151 (1)
145-151: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winPin the parser dependency for reproducibility.
setup.pydeclarestree-sitter-language-pack>=0.10.0, andCHANGELOG.mdrepeats the open range. Use the exact tested parser/binding versions in the dependency declaration, then update the changelog entry to match.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/PHASE1_SUMMARY.md` around lines 145 - 151, Pin the parser dependency to the exact tested parser and binding versions in setup.py’s install_requires declaration, replacing the open range. Update the corresponding dependency entry in CHANGELOG.md to use the same pinned versions; the references in docs/PHASE1_SUMMARY.md and CHANGELOG.md are documentation only and should match the final declaration.
🧹 Nitpick comments (8)
tests/test_tree_sitter_basic.py (1)
64-85: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the import relationships instead of printing them.
Lines 80-84 state that the test verifies the
osandsysimports, but they only print. The test passes if zero import relationships are produced. Add the assertion. Line 67 also uses an f-string with no placeholder (Ruff F541).💚 Proposed assertions
- print(f"\n📊 Analysis Summary:") + print("\n📊 Analysis Summary:")import_rels = [r for r in relationships if r.get("kind") == "imports"] + imported = {r["target_id"] for r in import_rels} + assert {"module::os", "module::sys"} <= imported, ( + f"Expected os/sys import relationships, got {sorted(imported)}" + )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_tree_sitter_basic.py` around lines 64 - 85, Update the import relationship verification in the test to assert that the collected `import_rels` contains the expected `os` and `sys` imports, rather than only printing them; keep the diagnostic output as needed. Also replace the placeholder-free summary f-string with a normal string literal to satisfy Ruff F541.Source: Linters/SAST tools
diffgraph/processing_modes/tree_sitter_dependency.py (3)
1036-1047: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDelete the deprecated legacy path instead of shipping it.
_analyze_changes_legacyhas no caller and is documented as reference-only. It keepsGraphManager,ChangeType,ComponentNode,_get_full_file_content, and_extract_function_callsalive only for dead code, and it carries its own lint findings (lines 1145, 1179). Git history preserves the implementation. Remove the method, the now-unused imports, and theself.graph_manager/self.current_file_componentsfields set in__init__.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@diffgraph/processing_modes/tree_sitter_dependency.py` around lines 1036 - 1047, Remove the deprecated _analyze_changes_legacy method and its dead-code dependencies, including GraphManager, ChangeType, ComponentNode, _get_full_file_content, and _extract_function_calls. Also remove the self.graph_manager and self.current_file_components assignments from __init__, while preserving the active analyze_changes_v2()/analyze_changes() paths.
15-24: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDo not attach attributes to the imported module.
tslp.ts = tsmutates the third-party module object for the whole process.tsis already in module scope, so the query code can callts.Queryandts.QueryCursordirectly. Also chain the re-raise so the original error is preserved (Ruff B904).♻️ Proposed import shim cleanup
try: import tree_sitter_language_pack as tslp import tree_sitter as ts - # Alias for convenience - tslp.ts = ts -except ImportError: +except ImportError as exc: raise ImportError( "tree-sitter-language-pack is required for tree-sitter-dependency-graph mode. " "Install it with: pip install tree-sitter-language-pack" - ) + ) from excThen replace
tslp.ts.Query/tslp.ts.QueryCursorwithts.Query/ts.QueryCursorat lines 198-199, 241-242, and 780-781.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@diffgraph/processing_modes/tree_sitter_dependency.py` around lines 15 - 24, Remove the `tslp.ts = ts` module mutation in the import shim, and re-raise the `ImportError` with exception chaining while preserving the existing message. Update the query construction and cursor usage in the relevant processing methods to call `ts.Query` and `ts.QueryCursor` directly instead of `tslp.ts.Query` and `tslp.ts.QueryCursor`.Source: Linters/SAST tools
816-831: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueDrop the unused
stagedparameter or handle staged files correctly.
_get_pre_change_contentdocuments both unstaged and staged diffs asgit show HEAD:<file>, then ignoresstaged. If the staged base actually needs:0:, parse it; otherwise, make this intent explicit by removingstaged.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@diffgraph/processing_modes/tree_sitter_dependency.py` around lines 816 - 831, Update _get_pre_change_content to remove the unused staged parameter if HEAD is intentionally the base for both staged and unstaged diffs, and revise its documentation and callers accordingly; otherwise, use staged to select the correct Git revision, including :0: when required for staged files.tests/test_tree_sitter_phase1.py (1)
88-93: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a real pre/post case, and remove the unused helper.
_make_processor_with_tmp_gitis never called, and its docstring describes mocking that it does not perform.Every test in this file uses
status="added"in an emptytmp_path, so both git lookups fail and the suppliedcontentfallback is used. The pre/post AST comparison introduced by this PR is therefore never exercised end to end:modified,deleted, andunchangedchange kinds are only covered by stubbed components intests/test_schema_v2_adapter.py.Add a fixture that initializes a git repository in
tmp_path, commitsSIMPLE_PYTHON_FILE, writesMODIFIED_PYTHON_FILE, and asserts the resulting change kinds and the parse-failure warning path. Do you want me to draft that fixture?🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_tree_sitter_phase1.py` around lines 88 - 93, Remove the unused _make_processor_with_tmp_git helper, then add an end-to-end test fixture that initializes git in tmp_path, commits SIMPLE_PYTHON_FILE, replaces it with MODIFIED_PYTHON_FILE, and runs the processor through the real pre/post lookup path. Assert the expected modified, deleted, and unchanged change kinds and cover the parse-failure warning path without relying on stubbed components.tests/test_schema_v2_adapter.py (1)
301-325: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse the
monkeypatchfixture for the socket patch.The manual assignment to
socket.socket.connectmutates global state for the whole test session if the restore is ever skipped.monkeypatchrestores the attribute automatically at teardown and removes the try/finally block.♻️ Proposed test simplification
- def test_no_network_calls(self): + def test_no_network_calls(self, monkeypatch): ... import socket original_connect = socket.socket.connect - calls = [] def mock_connect(self, *args, **kwargs): calls.append(args) return original_connect(self, *args, **kwargs) - socket.socket.connect = mock_connect - try: - self._minimal_output() - finally: - socket.socket.connect = original_connect + monkeypatch.setattr(socket.socket, "connect", mock_connect) + self._minimal_output()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_schema_v2_adapter.py` around lines 301 - 325, Update test_no_network_calls to accept pytest’s monkeypatch fixture and use it to replace socket.socket.connect, preserving the existing call recording and original-connect delegation. Remove the manual assignment, original_connect restoration, and try/finally block while keeping the assertion that _minimal_output() performs no connections.diffgraph/processing_modes/__init__.py (1)
96-101: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRecord why the tree-sitter processor was skipped, and fix the package name.
The comment names
tree-sitter-languages, buttree_sitter_dependency.pyrequirestree-sitter-language-pack. The barepassalso discards the install hint raised attree_sitter_dependency.pylines 20-24, and it hides real import errors fromschema_v2_adapterorgraph_manager. A user then sees only an unknown-mode failure later.♻️ Proposed optional-import handling
-# Import optional processors -try: - from . import tree_sitter_dependency # noqa: F401, E402 -except ImportError: - # tree-sitter-languages not installed, skip this processor - pass +# Import optional processors +try: + from . import tree_sitter_dependency # noqa: F401, E402 +except ImportError as exc: + # tree-sitter-language-pack not installed, or the module failed to import. + logger.debug("tree-sitter processor unavailable: %s", exc)Add
import loggingandlogger = logging.getLogger(__name__)at the top of the module.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@diffgraph/processing_modes/__init__.py` around lines 96 - 101, Update the optional processor import block in diffgraph.processing_modes to catch only the missing tree-sitter-language-pack dependency, log the skipped processor and installation hint through a module logger, and allow other ImportError causes from tree_sitter_dependency.py dependencies such as schema_v2_adapter or graph_manager to propagate. Add the requested logging import and module-level logger, and correct the package name in the skip message.diffgraph/processing_modes/schema_v2_adapter.py (1)
311-326: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winCarry the detected language into
FileEntry.
_build_file_entryemits only the four required fields. The processor already resolves the language attree_sitter_dependency.pyline 923, and the schema defineslanguagewithnullmeaning "static analysis not available for this file". Passing it through lets consumers distinguish a skipped file from a file with no symbols, without a second detection pass.
lines_addedandlines_removedare available from git diff metadata and are worth adding in the same change.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@diffgraph/processing_modes/schema_v2_adapter.py` around lines 311 - 326, Update _build_file_entry and its callers to carry the processor’s already-resolved language into each FileEntry, preserving null when static analysis is unavailable. Extend the internal file-change data passed to this function with lines_added and lines_removed from git diff metadata, and include both fields in the emitted entry without performing another language-detection pass.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@diffgraph/processing_modes/schema_v2_adapter.py`:
- Around line 243-273: Update build_import_relationship and its callers to
produce unique relationship IDs by passing an occurrence counter for each
(source_file, imported_module) pair and using the reserved multi-edge suffixing.
Ensure every source_id and target_id resolves to an emitted files[]/symbols[]
entity by adding module-kind symbol entries for import targets or using the
established unresolved-entity representation. Thread each import node’s line
through _extract_imports_python and the tree_sitter_dependency.py caller so
evidence includes line_start.
In `@diffgraph/processing_modes/tree_sitter_dependency.py`:
- Around line 947-976: Update
diffgraph/processing_modes/tree_sitter_dependency.py lines 947-976 to record
parse-failure warnings containing the file path and error text instead of
silently passing, and count unsupported-language skips from lines 924-927 in the
analysis metadata. Update diffgraph/processing_modes/schema_v2_adapter.py lines
360-376 so build_schema_v2_output accepts warnings, files_analyzed, and
files_skipped parameters and emits them rather than hardcoding an empty warnings
list; ensure the processor passes these values through.
- Around line 300-345: Update the non-Python extraction blocks, including the
TypeScript class and function extraction around language.query, to use the
current tree-sitter Query and QueryCursor APIs and normalize dict-based captures
before processing nodes. Apply the same change across Go, Rust, Java, and Swift
extractors so their existing symbol extraction paths populate components instead
of being swallowed by broad exception handlers; otherwise remove those languages
from the documented support.
In `@docs/GRAPH_EXPORT_FEATURE.md`:
- Around line 21-22: Update the “Export as pickle” documentation around the wild
diff example and load_graph_from_pickle() guidance to state that pickle loading
can execute code and must only use files from trusted local producers. Recommend
JSON or GraphML when exchanging artifacts between users or systems.
In `@docs/PHASE1_SUMMARY.md`:
- Around line 106-108: Update the file-content resolution documentation and
implementation referenced by “File Content Handling” to use diff provenance:
select HEAD:<path> for staged content, :0:<path> for unstaged content, and the
working tree for new files, with appropriate handling for renamed files. Expand
content-fetch tests to cover staged, unstaged, and renamed files while
preserving the existing fallback behavior for untracked files.
In `@docs/TESTING_GUIDE.md`:
- Around line 9-11: Update docs/TESTING_GUIDE.md at lines 9-11, 24-26, 44-46,
121-124, 177-189, and 297-302 to use repository-root-relative paths:
tests/test_cli_manual.sh, tests/test_graph_export.py, and
tests/examples/example_usage.py as applicable. Update tests/test_cli_manual.sh
lines 101-106 so example_usage.py is resolved relative to the repository or
script directory rather than the caller’s working directory.
In `@README.md`:
- Around line 126-128: Update the shared graph-export call to use the selected
processor instead of agent.graph_manager, keeping all examples consistent:
README.md lines 126-128, docs/TESTING_GUIDE.md lines 38-40, and
docs/TESTING_GUIDE.md lines 115-118.
- Around line 121-124: Update the README tree-sitter example to use the existing
HTML adapter rather than invoking the incompatible direct CLI path. First align
the tree-sitter analysis output with the HTML generation contract by wiring a
schema v2 adapter or updating the relevant HTML-generation flow, then document
and test the command so it completes successfully without hanging.
In `@setup.py`:
- Line 9: Update the Python compatibility metadata in setup.py to match the
required tree-sitter-language-pack dependency: set python_requires to >=3.10 and
remove the Python 3.7–3.9 classifiers. Only remove the dependency instead if
tree-sitter mode is intentionally optional and the package can install without
it.
In `@tests/test_cli_manual.sh`:
- Around line 101-106: Update the Test 4 block around the example usage command
to capture python example_usage.py test_output.json output in a temporary file
before invoking head. Ensure the Python command runs independently under set -e
so its failure stops the script, then display the captured output with head and
preserve the existing success message only after both steps succeed.
- Around line 121-131: Update the cleanup flow in the manual harness so it never
blocks on read by default: remove the interactive read and delete the generated
test files automatically. If inspection is needed, support an explicit --keep
option that skips removal and reports the files were retained.
- Around line 46-49: Align schema examples and validation with v2: in
tests/test_cli_manual.sh lines 46-49, exercise structured JSON export or add a
separate v2 export case; in tests/test_cli_manual.sh lines 61-71, assert version
2.0 plus metadata, categorized files, source-code files/components, and
relationship edges; in docs/TESTING_GUIDE.md lines 225-256, replace the version
1.0 example or explicitly label it as legacy output.
- Around line 109-113: Update the Test 5 CLI help check in
tests/test_cli_manual.sh so missing --format or --graph-format options produce a
nonzero exit status instead of succeeding through the || branch. Preserve the
success message when grep finds the required options, and explicitly exit or
return failure after printing the error message.
In `@tests/test_graph_export.py`:
- Around line 46-47: Replace fixed working-directory export filenames with paths
created under a test-local temporary directory for each affected test. Apply
this to tests/test_graph_export.py lines 46-47, 69-70, and 84-85, and
tests/test_structured_export.py lines 118-119, 239-240, and 267-268, reusing the
test framework’s temporary-directory fixture and preserving each export format
and assertion flow.
- Around line 59-65: The graph export tests currently validate only node counts
or format markers instead of semantic round-trips. Extend the JSON and pickle
checks around load_graph_from_json and their corresponding loaders to compare
component edges, dependency/dependent lists, processed_files, file status,
summaries, and processed-file metadata against the original graph; load the
GraphML output with the NetworkX GraphML loader and perform equivalent semantic
comparisons rather than only checking XML/header content.
In `@tests/test_structured_export.py`:
- Around line 124-213: Update the structured export validation test to load the
v2 JSON Schema and call jsonschema.validate() on the parsed data before
field-level assertions, matching test_tree_sitter_phase1.py. Extend coverage
with canonical provenance, range, and rename fixtures or assertions so
validation checks schema semantics rather than only fixed counts. Preserve the
existing structural checks while ensuring all representative exports conform to
the schema contract.
---
Minor comments:
In `@CHANGELOG.md`:
- Around line 11-16: Update the “Tree-sitter Dependency Extraction (Phase 1)”
changelog entry to clearly distinguish Python as fully supported, while
describing TypeScript, JavaScript, Go, Rust, Java, and Swift as framework/parser
work in progress.
In `@diffgraph/processing_modes/tree_sitter_dependency.py`:
- Around line 746-761: The _extract_components dispatch must handle tsx and stop
using TypeScript queries for JavaScript. Pass the detected language through the
relevant extraction flow, route tsx appropriately, and update
_extract_components_typescript or its grammar-loading logic to use the matching
JavaScript, TypeScript, or TSX grammar so query node types align with the parser
returned by _get_parser.
In `@docs/GRAPH_EXPORT_FEATURE.md`:
- Around line 164-180: Update the Testing commands in GRAPH_EXPORT_FEATURE.md to
use repository-relative paths for tests/test_graph_export.py and
tests/examples/example_usage.py, while leaving the wild diff example unchanged.
In `@docs/PHASE1_SUMMARY.md`:
- Around line 186-190: Replace the hard-coded developer-specific path in the
“Priority 3: Testing & Validation” section with a portable repository-relative
example or an environment-variable-based location, while preserving the
instruction to use real repositories for validation.
- Around line 131-136: Update the extracted-components example in PHASE1_SUMMARY
so the expected output matches the sample input: remove another_function from
the “Output components extracted” list, unless the input is explicitly updated
to define it.
- Around line 82-95: The Tree-sitter documentation example in “Tree-sitter API
(v0.25+)” uses the wrong parser construction. Update it to demonstrate the same
ts.Parser-based initialization used by TreeSitterProcessor._get_parser(), and
align any nearby dependency/API guidance with that implementation instead of
tslp.get_parser(language).
- Line 154: Update the Python compatibility note in PHASE1_SUMMARY.md to remove
the claim that the project runs on Python 3.13, while retaining the
tree-sitter-language-pack requirement and only stating supported versions backed
by setup.py or project metadata.
- Around line 145-151: Pin the parser dependency to the exact tested parser and
binding versions in setup.py’s install_requires declaration, replacing the open
range. Update the corresponding dependency entry in CHANGELOG.md to use the same
pinned versions; the references in docs/PHASE1_SUMMARY.md and CHANGELOG.md are
documentation only and should match the final declaration.
---
Nitpick comments:
In `@diffgraph/processing_modes/__init__.py`:
- Around line 96-101: Update the optional processor import block in
diffgraph.processing_modes to catch only the missing tree-sitter-language-pack
dependency, log the skipped processor and installation hint through a module
logger, and allow other ImportError causes from tree_sitter_dependency.py
dependencies such as schema_v2_adapter or graph_manager to propagate. Add the
requested logging import and module-level logger, and correct the package name
in the skip message.
In `@diffgraph/processing_modes/schema_v2_adapter.py`:
- Around line 311-326: Update _build_file_entry and its callers to carry the
processor’s already-resolved language into each FileEntry, preserving null when
static analysis is unavailable. Extend the internal file-change data passed to
this function with lines_added and lines_removed from git diff metadata, and
include both fields in the emitted entry without performing another
language-detection pass.
In `@diffgraph/processing_modes/tree_sitter_dependency.py`:
- Around line 1036-1047: Remove the deprecated _analyze_changes_legacy method
and its dead-code dependencies, including GraphManager, ChangeType,
ComponentNode, _get_full_file_content, and _extract_function_calls. Also remove
the self.graph_manager and self.current_file_components assignments from
__init__, while preserving the active analyze_changes_v2()/analyze_changes()
paths.
- Around line 15-24: Remove the `tslp.ts = ts` module mutation in the import
shim, and re-raise the `ImportError` with exception chaining while preserving
the existing message. Update the query construction and cursor usage in the
relevant processing methods to call `ts.Query` and `ts.QueryCursor` directly
instead of `tslp.ts.Query` and `tslp.ts.QueryCursor`.
- Around line 816-831: Update _get_pre_change_content to remove the unused
staged parameter if HEAD is intentionally the base for both staged and unstaged
diffs, and revise its documentation and callers accordingly; otherwise, use
staged to select the correct Git revision, including :0: when required for
staged files.
In `@tests/test_schema_v2_adapter.py`:
- Around line 301-325: Update test_no_network_calls to accept pytest’s
monkeypatch fixture and use it to replace socket.socket.connect, preserving the
existing call recording and original-connect delegation. Remove the manual
assignment, original_connect restoration, and try/finally block while keeping
the assertion that _minimal_output() performs no connections.
In `@tests/test_tree_sitter_basic.py`:
- Around line 64-85: Update the import relationship verification in the test to
assert that the collected `import_rels` contains the expected `os` and `sys`
imports, rather than only printing them; keep the diagnostic output as needed.
Also replace the placeholder-free summary f-string with a normal string literal
to satisfy Ruff F541.
In `@tests/test_tree_sitter_phase1.py`:
- Around line 88-93: Remove the unused _make_processor_with_tmp_git helper, then
add an end-to-end test fixture that initializes git in tmp_path, commits
SIMPLE_PYTHON_FILE, replaces it with MODIFIED_PYTHON_FILE, and runs the
processor through the real pre/post lookup path. Assert the expected modified,
deleted, and unchanged change kinds and cover the parse-failure warning path
without relying on stubbed components.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 66020436-b723-45aa-a806-459de0297732
📒 Files selected for processing (17)
CHANGELOG.mdREADME.mddiffgraph/processing_modes/__init__.pydiffgraph/processing_modes/schema_v2_adapter.pydiffgraph/processing_modes/tree_sitter_dependency.pydiffgraph/schema/diffgraph-v2.schema.jsondocs/GRAPH_EXPORT_FEATURE.mddocs/PHASE1_SUMMARY.mddocs/TESTING_GUIDE.mdsetup.pytests/examples/example_usage.pytests/test_cli_manual.shtests/test_graph_export.pytests/test_schema_v2_adapter.pytests/test_structured_export.pytests/test_tree_sitter_basic.pytests/test_tree_sitter_phase1.py
| # Extract classes | ||
| try: | ||
| query = language.query(class_query) | ||
| captures = query.captures(root) | ||
|
|
||
| for node, capture_name in captures: | ||
| if capture_name == "class_name": | ||
| class_name = source_bytes[node.start_byte:node.end_byte].decode('utf-8') | ||
| class_node = node.parent | ||
|
|
||
| components.append(ExtractedComponent( | ||
| name=class_name, | ||
| component_type="container", | ||
| start_line=class_node.start_point[0], | ||
| end_line=class_node.end_point[0], | ||
| file_path=file_path | ||
| )) | ||
|
|
||
| # Extract methods | ||
| for child in class_node.children: | ||
| if child.type == "class_body": | ||
| for member in child.children: | ||
| if member.type in ["method_definition", "public_field_definition"]: | ||
| name_node = None | ||
| for m_child in member.children: | ||
| if m_child.type == "property_identifier": | ||
| name_node = m_child | ||
| break | ||
|
|
||
| if name_node: | ||
| method_name = source_bytes[name_node.start_byte:name_node.end_byte].decode('utf-8') | ||
| components.append(ExtractedComponent( | ||
| name=method_name, | ||
| component_type="method", | ||
| parent=class_name, | ||
| start_line=member.start_point[0], | ||
| end_line=member.end_point[0], | ||
| file_path=file_path | ||
| )) | ||
| except Exception as e: | ||
| print(f"Warning: Error extracting TypeScript classes: {e}") | ||
|
|
||
| # Extract functions | ||
| try: | ||
| query = language.query(function_query) | ||
| captures = query.captures(root) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: List every legacy tree-sitter query call site and captures-unpacking pattern.
set -euo pipefail
fd -e py . | xargs rg -n -C2 '\.query\(|QueryCursor\(|ts\.Query\(|for .*, capture_name in'
# Confirm which language extractors are advertised versus migrated.
rg -n 'Supports:|Supports Python' --glob '*.py' --glob '*.md'Repository: WildestAI/DiffGraph-CLI
Length of output: 10577
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## Python dependency/version files"
git ls-files | rg '(^|/)(pyproject.toml|requirements.*\.txt|Pipfile|poetry.lock|uv.lock|setup.py|setup.cfg)$' || true
echo
echo "## tree-sitter references"
rg -n 'tree-sitter|tslp|py_tree_sitter|ts\.Query|QueryCursor|language\.query' pyproject.toml requirements*.txt Pipfile poetry.lock uv.lock setup.py setup.cfg diffgraph/processing_modes/tree_sitter_dependency.py 2>/dev/null || true
echo
echo "## Relevant tree_sitter_dependency.py sections"
sed -n '80,140p' diffgraph/processing_modes/tree_sitter_dependency.py
sed -n '180,255p' diffgraph/processing_modes/tree_sitter_dependency.py
sed -n '288,355p' diffgraph/processing_modes/tree_sitter_dependency.py
sed -n '390,475p' diffgraph/processing_modes/tree_sitter_dependency.py
sed -n '505,545p' diffgraph/processing_modes/tree_sitter_dependency.py
sed -n '592,620p' diffgraph/processing_modes/tree_sitter_dependency.py
sed -n '662,725p' diffgraph/processing_modes/tree_sitter_dependency.py
sed -n '770,790p' diffgraph/processing_modes/tree_sitter_dependency.pyRepository: WildestAI/DiffGraph-CLI
Length of output: 23998
🌐 Web query:
py-tree-sitter 0.25 Language query deprecated QueryCursor captures return dict
💡 Result:
In py-tree-sitter version 0.25.0 and later, the query execution methods were moved from the Query class to the QueryCursor class [1][2]. The methods Query.captures and Query.matches were removed [3], and you must now use QueryCursor to perform these operations [4][5]. The QueryCursor.captures method continues to return a dictionary where the keys are capture names and the values are lists of the captured nodes [6][4]. Migration details: - Instead of calling query.captures(node), you must instantiate a QueryCursor with your query and then call captures on the cursor instance [4]. Example of the updated usage: from tree_sitter import Query, QueryCursor query = Query(language, "your_query_string") cursor = QueryCursor(query) # Returns a dict of capture_name: [nodes] captures = cursor.captures(tree.root_node) # To perform matches: # Returns a list of (match_index, {capture_name: [nodes]}) matches = cursor.matches(tree.root_node) This change was part of a broader refactor in v0.25.0 that introduced QueryCursor to handle query execution, timeouts, and match limits, which were previously managed directly on the Query object [2][5].
Citations:
- 1: https://github.com/tree-sitter/py-tree-sitter/releases/tag/v0.25.0
- 2: feat!: ABI 15 & QueryCursor tree-sitter/py-tree-sitter#333
- 3: cdeust/ai-architect-mcp@78ed987
- 4: https://pypi.org/project/tree-sitter/0.25.2/
- 5: https://github.com/tree-sitter/py-tree-sitter/releases
- 6: https://tree-sitter.github.io/py-tree-sitter/classes/tree_sitter.QueryCursor.html
Fix the legacy tree-sitter query calls so TypeScript and other extracted languages return symbols.
The non-Python extractors still use language.query(...) and iterate for node, capture_name in captures, but the current tree-sitter-API path uses ts.Query(...), QueryCursor(...), and dict captures. Those blocks catch all exceptions and return no components, so TypeScript, Go, Rust, Java, and Swift runs advertise support while silently returning zero symbols. Update those extractor calls to the current API or reduce the documented supported languages.
🧰 Tools
🪛 Ruff (0.16.0)
[warning] 339-339: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@diffgraph/processing_modes/tree_sitter_dependency.py` around lines 300 - 345,
Update the non-Python extraction blocks, including the TypeScript class and
function extraction around language.query, to use the current tree-sitter Query
and QueryCursor APIs and normalize dict-based captures before processing nodes.
Apply the same change across Go, Rust, Java, and Swift extractors so their
existing symbol extraction paths populate components instead of being swallowed
by broad exception handlers; otherwise remove those languages from the
documented support.
There was a problem hiding this comment.
Confirmed still valid on current head 01d786f.
Python and import extraction use ts.Query + ts.QueryCursor with dict captures (lines 199–244 and 781–783), but every non-Python extractor still calls the removed language.query(...)/query.captures(...) API and tuple-unpacks captures: TypeScript/JS 303–346, Go 406–447, Rust 516–537, Java 606–607, and Swift 675–718. Each path catches Exception, so the advertised languages can return zero symbols instead of failing clearly. The repository itself also records this migration as unfinished in docs/PHASE1_SUMMARY.md lines 172–176.
Please either migrate these extractors to the same QueryCursor/dict-capture helper and add one symbol fixture per advertised language, or narrow TreeSitterProcessor metadata/docs to Python-only for this PR. I’m leaving this thread unresolved. I did not push because GitHub reports maintainerCanModify=false for this contributor-owned branch.
There was a problem hiding this comment.
@nia-sg-bot, acknowledged. The finding remains valid and unresolved.
The processor must not advertise TypeScript, JavaScript, Go, Rust, Java, or Swift support while their extractors use the obsolete query API and suppress extraction failures. The PR should either migrate every affected extractor to ts.Query and ts.QueryCursor with dict captures, with a symbol fixture for each language, or restrict the processor metadata and documentation to Python.
✏️ Learnings added
Learnt from: nia-sg-bot
Repo: WildestAI/DiffGraph-CLI PR: 13
File: diffgraph/processing_modes/tree_sitter_dependency.py:301-346
Timestamp: 2026-08-03T20:02:13.002Z
Learning: In `diffgraph/processing_modes/tree_sitter_dependency.py`, non-Python Tree-sitter extractors must use `ts.Query` and `ts.QueryCursor` with dict-based captures. The removed `language.query(...)/query.captures(...)` API causes TypeScript, JavaScript, Go, Rust, Java, and Swift extraction to fail silently when broad exception handlers catch the error. Do not advertise these languages as supported until their extractors are migrated and covered by symbol fixtures.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
You are interacting with an AI system.
| ### File Content Handling | ||
|
|
||
| Uses `git show HEAD:path` to get full file content, falling back to filesystem for untracked files. This ensures we analyze complete context, not just diffs. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n 'git show|HEAD:|blob|rename|provenance|diff_args' diffgraph testsRepository: WildestAI/DiffGraph-CLI
Length of output: 5640
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '--- relevant docs ---\n'
sed -n '96,114p' docs/PHASE1_SUMMARY.md
printf '\n--- utils/git_utils.py ---\n'
wc -l diffgraph/utils/git_utils.py
sed -n '1,135p' diffgraph/utils/git_utils.py
printf '\n--- cli.py relevant sections ---\n'
sed -n '1,130p' diffgraph/cli.py
printf '\n--- tree_sitter_dependency.py relevant section ---\n'
sed -n '148,180p' diffgraph/processing_modes/tree_sitter_dependency.py
sed -n '800,855p' diffgraph/processing_modes/tree_sitter_dependency.py
sed -n '870,925p' diffgraph/processing_modes/tree_sitter_dependency.py
printf '\n--- schema adapter relevant ---\n'
sed -n '128,150p' diffgraph/processing_modes/schema_v2_adapter.py
printf '\n--- structured export rename notes ---\n'
sed -n '218,232p' diffgraph/structured_export.pyRepository: WildestAI/DiffGraph-CLI
Length of output: 252
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo '--- relevant docs ---'
sed -n '96,114p' docs/PHASE1_SUMMARY.md
echo '--- utils/git_utils.py ---'
wc -l diffgraph/utils/git_utils.py
sed -n '1,135p' diffgraph/utils/git_utils.py
echo '--- cli.py relevant sections ---'
sed -n '1,130p' diffgraph/cli.py
echo '--- tree_sitter_dependency.py relevant section ---'
sed -n '148,180p' diffgraph/processing_modes/tree_sitter_dependency.py
sed -n '800,855p' diffgraph/processing_modes/tree_sitter_dependency.py
sed -n '870,925p' diffgraph/processing_modes/tree_sitter_dependency.py
echo '--- schema adapter relevant ---'
sed -n '128,150p' diffgraph/processing_modes/schema_v2_adapter.py
echo '--- structured export rename notes ---'
sed -n '218,232p' diffgraph/structured_export.pyRepository: WildestAI/DiffGraph-CLI
Length of output: 16832
Use the actual diff provenance for content resolution.
The docs claim git show HEAD:path provides the file content for analysis, but unstaged working-tree diffs use the modified working-tree filesystem content. Resolve changed files by diff status/source and select HEAD:<path>/:0:<path>/working-tree/new-file fallback accordingly. Include staged, unstaged, and renamed files in the content-fetch tests.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/PHASE1_SUMMARY.md` around lines 106 - 108, Update the file-content
resolution documentation and implementation referenced by “File Content
Handling” to use diff provenance: select HEAD:<path> for staged content,
:0:<path> for unstaged content, and the working tree for new files, with
appropriate handling for renamed files. Expand content-fetch tests to cover
staged, unstaged, and renamed files while preserving the existing fallback
behavior for untracked files.
There was a problem hiding this comment.
Confirmed still valid on current head 01d786f and direction-blocking for the canonical artifact.
_get_pre_change_content() reads HEAD:<path> for both staged and unstaged changes (lines 817–832), while _get_post_change_content() reads :0:<path> only for staged and the filesystem for unstaged (834–856). The analyzer accepts a display-level diff_ref, but it does not bind each file to immutable pre/post object IDs, preserve a rename's old path, or distinguish an unstaged preimage from HEAD when the index already differs. That can compare the wrong pair for partially staged files and makes the result non-reproducible.
Please keep this unresolved until the PR has status-aware old/new paths plus exact pre/post blob identities and fixtures for: staged modify/add/delete/rename; unstaged changes on top of a different index blob; and unusual filenames. The extension and website currently advertise staged/unstaged commands but contain no schema-v2 provenance fixture, so this producer contract needs to be correct before either consumer can claim compatibility.
There was a problem hiding this comment.
@nia-sg-bot, agreed. I will keep this finding unresolved.
The current content selection does not preserve the immutable preimage and postimage required for a canonical schema v2 artifact. In particular, an unstaged change must use the index blob as its preimage when the index differs from HEAD.
Resolution requires:
- Status-aware old and new paths, including rename source paths.
- Exact preimage and postimage blob identities in the produced provenance.
- Correct handling for staged and unstaged modify, add, delete, and rename cases.
- Fixtures for unstaged changes over a different index blob and unusual filenames.
- Schema v2 provenance tests before staged or unstaged consumers claim compatibility.
🐇
✏️ Learnings added
Learnt from: nia-sg-bot
Repo: WildestAI/DiffGraph-CLI PR: 13
File: docs/PHASE1_SUMMARY.md:106-108
Timestamp: 2026-08-04T00:02:30.901Z
Learning: For DiffGraph schema v2 artifacts, `TreeSitterProcessor` content resolution must preserve status-aware old and new paths and immutable preimage and postimage blob identities. For an unstaged change where the index differs from `HEAD`, the index blob is the preimage; using `HEAD:<path>` produces an incorrect comparison. Rename source paths and staged/unstaged add, delete, modify, rename, and unusual-filename fixtures are required for reproducible provenance.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
You are interacting with an AI system.
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 16
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (8)
docs/GRAPH_EXPORT_FEATURE.md-164-180 (1)
164-180: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse repository-relative script paths.
The documented commands use files as if they are in the current directory. The reviewed files are
tests/test_graph_export.pyandtests/examples/example_usage.py. These commands fail when users run them from the repository root.Proposed documentation fix
- python test_graph_export.py + python tests/test_graph_export.py ... - python example_usage.py my-changes.json + python tests/examples/example_usage.py my-changes.json🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/GRAPH_EXPORT_FEATURE.md` around lines 164 - 180, Update the Testing commands in GRAPH_EXPORT_FEATURE.md to use repository-relative paths for tests/test_graph_export.py and tests/examples/example_usage.py, while leaving the wild diff example unchanged.diffgraph/processing_modes/tree_sitter_dependency.py-746-761 (1)
746-761: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDispatch drops
tsxand mixes grammars for JavaScript.
LANGUAGE_CONFIGSdeclarestsxat lines 65-68, but_extract_componentshas notsxbranch, so.tsxfiles fall through toreturn []. The file is still counted inlanguages_detectedand reported with zero symbols.
javascriptis routed to_extract_components_typescript, which builds its queries from the TypeScript grammar at line 283 while_get_parserparses the tree with the JavaScript grammar. The node types differ; JavaScript class names areidentifier, nottype_identifier. Pass the detected language into the extractor and load the matching grammar.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@diffgraph/processing_modes/tree_sitter_dependency.py` around lines 746 - 761, The _extract_components dispatch must handle tsx and stop using TypeScript queries for JavaScript. Pass the detected language through the relevant extraction flow, route tsx appropriately, and update _extract_components_typescript or its grammar-loading logic to use the matching JavaScript, TypeScript, or TSX grammar so query node types align with the parser returned by _get_parser.CHANGELOG.md-11-16 (1)
11-16: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winClarify the implemented language status.
The entry lists TypeScript, JavaScript, Go, Rust, Java, and Swift beside Python without saying that they are incomplete. State that Python is fully supported and the other languages are framework or parser work in progress.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@CHANGELOG.md` around lines 11 - 16, Update the “Tree-sitter Dependency Extraction (Phase 1)” changelog entry to clearly distinguish Python as fully supported, while describing TypeScript, JavaScript, Go, Rust, Java, and Swift as framework/parser work in progress.docs/PHASE1_SUMMARY.md-186-190 (1)
186-190: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winReplace the developer-specific absolute path.
/Users/apple/Work/Personal/opensourceis not portable and exposes a local filesystem layout. Use a repository-relative example or an environment variable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/PHASE1_SUMMARY.md` around lines 186 - 190, Replace the hard-coded developer-specific path in the “Priority 3: Testing & Validation” section with a portable repository-relative example or an environment-variable-based location, while preserving the instruction to use real repositories for validation.docs/PHASE1_SUMMARY.md-131-136 (1)
131-136: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRemove the component that is not in the input.
The sample input defines
MyClass,__init__,increment, andstandalone_function. It does not defineanother_function. Add that function to the input or remove it from the expected output.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/PHASE1_SUMMARY.md` around lines 131 - 136, Update the extracted-components example in PHASE1_SUMMARY so the expected output matches the sample input: remove another_function from the “Output components extracted” list, unless the input is explicitly updated to define it.docs/PHASE1_SUMMARY.md-82-95 (1)
82-95: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse
ts.Parser(...)for the Tree-sitter parser example/pin.The implementation creates the parser with
ts.Parser(tslp.get_language(...)), nottslp.get_parser(language). Keep the docs and dependency declarations aligned with the API used byTreeSitterProcessor._get_parser().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/PHASE1_SUMMARY.md` around lines 82 - 95, The Tree-sitter documentation example in “Tree-sitter API (v0.25+)” uses the wrong parser construction. Update it to demonstrate the same ts.Parser-based initialization used by TreeSitterProcessor._get_parser(), and align any nearby dependency/API guidance with that implementation instead of tslp.get_parser(language).docs/PHASE1_SUMMARY.md-154-154 (1)
154-154: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove the unsupported runtime claim.
setup.pydeclarespython_requires=">=3.7"and no CI or package metadata declares Python 3.13. Keep thetree-sitter-language-packrequirement, but do not assert that the project runs on Python 3.13.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/PHASE1_SUMMARY.md` at line 154, Update the Python compatibility note in PHASE1_SUMMARY.md to remove the claim that the project runs on Python 3.13, while retaining the tree-sitter-language-pack requirement and only stating supported versions backed by setup.py or project metadata.docs/PHASE1_SUMMARY.md-145-151 (1)
145-151: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winPin the parser dependency for reproducibility.
setup.pydeclarestree-sitter-language-pack>=0.10.0, andCHANGELOG.mdrepeats the open range. Use the exact tested parser/binding versions in the dependency declaration, then update the changelog entry to match.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/PHASE1_SUMMARY.md` around lines 145 - 151, Pin the parser dependency to the exact tested parser and binding versions in setup.py’s install_requires declaration, replacing the open range. Update the corresponding dependency entry in CHANGELOG.md to use the same pinned versions; the references in docs/PHASE1_SUMMARY.md and CHANGELOG.md are documentation only and should match the final declaration.
🧹 Nitpick comments (8)
tests/test_tree_sitter_basic.py (1)
64-85: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the import relationships instead of printing them.
Lines 80-84 state that the test verifies the
osandsysimports, but they only print. The test passes if zero import relationships are produced. Add the assertion. Line 67 also uses an f-string with no placeholder (Ruff F541).💚 Proposed assertions
- print(f"\n📊 Analysis Summary:") + print("\n📊 Analysis Summary:")import_rels = [r for r in relationships if r.get("kind") == "imports"] + imported = {r["target_id"] for r in import_rels} + assert {"module::os", "module::sys"} <= imported, ( + f"Expected os/sys import relationships, got {sorted(imported)}" + )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_tree_sitter_basic.py` around lines 64 - 85, Update the import relationship verification in the test to assert that the collected `import_rels` contains the expected `os` and `sys` imports, rather than only printing them; keep the diagnostic output as needed. Also replace the placeholder-free summary f-string with a normal string literal to satisfy Ruff F541.Source: Linters/SAST tools
diffgraph/processing_modes/tree_sitter_dependency.py (3)
1036-1047: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDelete the deprecated legacy path instead of shipping it.
_analyze_changes_legacyhas no caller and is documented as reference-only. It keepsGraphManager,ChangeType,ComponentNode,_get_full_file_content, and_extract_function_callsalive only for dead code, and it carries its own lint findings (lines 1145, 1179). Git history preserves the implementation. Remove the method, the now-unused imports, and theself.graph_manager/self.current_file_componentsfields set in__init__.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@diffgraph/processing_modes/tree_sitter_dependency.py` around lines 1036 - 1047, Remove the deprecated _analyze_changes_legacy method and its dead-code dependencies, including GraphManager, ChangeType, ComponentNode, _get_full_file_content, and _extract_function_calls. Also remove the self.graph_manager and self.current_file_components assignments from __init__, while preserving the active analyze_changes_v2()/analyze_changes() paths.
15-24: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDo not attach attributes to the imported module.
tslp.ts = tsmutates the third-party module object for the whole process.tsis already in module scope, so the query code can callts.Queryandts.QueryCursordirectly. Also chain the re-raise so the original error is preserved (Ruff B904).♻️ Proposed import shim cleanup
try: import tree_sitter_language_pack as tslp import tree_sitter as ts - # Alias for convenience - tslp.ts = ts -except ImportError: +except ImportError as exc: raise ImportError( "tree-sitter-language-pack is required for tree-sitter-dependency-graph mode. " "Install it with: pip install tree-sitter-language-pack" - ) + ) from excThen replace
tslp.ts.Query/tslp.ts.QueryCursorwithts.Query/ts.QueryCursorat lines 198-199, 241-242, and 780-781.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@diffgraph/processing_modes/tree_sitter_dependency.py` around lines 15 - 24, Remove the `tslp.ts = ts` module mutation in the import shim, and re-raise the `ImportError` with exception chaining while preserving the existing message. Update the query construction and cursor usage in the relevant processing methods to call `ts.Query` and `ts.QueryCursor` directly instead of `tslp.ts.Query` and `tslp.ts.QueryCursor`.Source: Linters/SAST tools
816-831: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueDrop the unused
stagedparameter or handle staged files correctly.
_get_pre_change_contentdocuments both unstaged and staged diffs asgit show HEAD:<file>, then ignoresstaged. If the staged base actually needs:0:, parse it; otherwise, make this intent explicit by removingstaged.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@diffgraph/processing_modes/tree_sitter_dependency.py` around lines 816 - 831, Update _get_pre_change_content to remove the unused staged parameter if HEAD is intentionally the base for both staged and unstaged diffs, and revise its documentation and callers accordingly; otherwise, use staged to select the correct Git revision, including :0: when required for staged files.tests/test_tree_sitter_phase1.py (1)
88-93: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a real pre/post case, and remove the unused helper.
_make_processor_with_tmp_gitis never called, and its docstring describes mocking that it does not perform.Every test in this file uses
status="added"in an emptytmp_path, so both git lookups fail and the suppliedcontentfallback is used. The pre/post AST comparison introduced by this PR is therefore never exercised end to end:modified,deleted, andunchangedchange kinds are only covered by stubbed components intests/test_schema_v2_adapter.py.Add a fixture that initializes a git repository in
tmp_path, commitsSIMPLE_PYTHON_FILE, writesMODIFIED_PYTHON_FILE, and asserts the resulting change kinds and the parse-failure warning path. Do you want me to draft that fixture?🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_tree_sitter_phase1.py` around lines 88 - 93, Remove the unused _make_processor_with_tmp_git helper, then add an end-to-end test fixture that initializes git in tmp_path, commits SIMPLE_PYTHON_FILE, replaces it with MODIFIED_PYTHON_FILE, and runs the processor through the real pre/post lookup path. Assert the expected modified, deleted, and unchanged change kinds and cover the parse-failure warning path without relying on stubbed components.tests/test_schema_v2_adapter.py (1)
301-325: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse the
monkeypatchfixture for the socket patch.The manual assignment to
socket.socket.connectmutates global state for the whole test session if the restore is ever skipped.monkeypatchrestores the attribute automatically at teardown and removes the try/finally block.♻️ Proposed test simplification
- def test_no_network_calls(self): + def test_no_network_calls(self, monkeypatch): ... import socket original_connect = socket.socket.connect - calls = [] def mock_connect(self, *args, **kwargs): calls.append(args) return original_connect(self, *args, **kwargs) - socket.socket.connect = mock_connect - try: - self._minimal_output() - finally: - socket.socket.connect = original_connect + monkeypatch.setattr(socket.socket, "connect", mock_connect) + self._minimal_output()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_schema_v2_adapter.py` around lines 301 - 325, Update test_no_network_calls to accept pytest’s monkeypatch fixture and use it to replace socket.socket.connect, preserving the existing call recording and original-connect delegation. Remove the manual assignment, original_connect restoration, and try/finally block while keeping the assertion that _minimal_output() performs no connections.diffgraph/processing_modes/__init__.py (1)
96-101: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRecord why the tree-sitter processor was skipped, and fix the package name.
The comment names
tree-sitter-languages, buttree_sitter_dependency.pyrequirestree-sitter-language-pack. The barepassalso discards the install hint raised attree_sitter_dependency.pylines 20-24, and it hides real import errors fromschema_v2_adapterorgraph_manager. A user then sees only an unknown-mode failure later.♻️ Proposed optional-import handling
-# Import optional processors -try: - from . import tree_sitter_dependency # noqa: F401, E402 -except ImportError: - # tree-sitter-languages not installed, skip this processor - pass +# Import optional processors +try: + from . import tree_sitter_dependency # noqa: F401, E402 +except ImportError as exc: + # tree-sitter-language-pack not installed, or the module failed to import. + logger.debug("tree-sitter processor unavailable: %s", exc)Add
import loggingandlogger = logging.getLogger(__name__)at the top of the module.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@diffgraph/processing_modes/__init__.py` around lines 96 - 101, Update the optional processor import block in diffgraph.processing_modes to catch only the missing tree-sitter-language-pack dependency, log the skipped processor and installation hint through a module logger, and allow other ImportError causes from tree_sitter_dependency.py dependencies such as schema_v2_adapter or graph_manager to propagate. Add the requested logging import and module-level logger, and correct the package name in the skip message.diffgraph/processing_modes/schema_v2_adapter.py (1)
311-326: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winCarry the detected language into
FileEntry.
_build_file_entryemits only the four required fields. The processor already resolves the language attree_sitter_dependency.pyline 923, and the schema defineslanguagewithnullmeaning "static analysis not available for this file". Passing it through lets consumers distinguish a skipped file from a file with no symbols, without a second detection pass.
lines_addedandlines_removedare available from git diff metadata and are worth adding in the same change.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@diffgraph/processing_modes/schema_v2_adapter.py` around lines 311 - 326, Update _build_file_entry and its callers to carry the processor’s already-resolved language into each FileEntry, preserving null when static analysis is unavailable. Extend the internal file-change data passed to this function with lines_added and lines_removed from git diff metadata, and include both fields in the emitted entry without performing another language-detection pass.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@diffgraph/processing_modes/schema_v2_adapter.py`:
- Around line 243-273: Update build_import_relationship and its callers to
produce unique relationship IDs by passing an occurrence counter for each
(source_file, imported_module) pair and using the reserved multi-edge suffixing.
Ensure every source_id and target_id resolves to an emitted files[]/symbols[]
entity by adding module-kind symbol entries for import targets or using the
established unresolved-entity representation. Thread each import node’s line
through _extract_imports_python and the tree_sitter_dependency.py caller so
evidence includes line_start.
In `@diffgraph/processing_modes/tree_sitter_dependency.py`:
- Around line 947-976: Update
diffgraph/processing_modes/tree_sitter_dependency.py lines 947-976 to record
parse-failure warnings containing the file path and error text instead of
silently passing, and count unsupported-language skips from lines 924-927 in the
analysis metadata. Update diffgraph/processing_modes/schema_v2_adapter.py lines
360-376 so build_schema_v2_output accepts warnings, files_analyzed, and
files_skipped parameters and emits them rather than hardcoding an empty warnings
list; ensure the processor passes these values through.
- Around line 300-345: Update the non-Python extraction blocks, including the
TypeScript class and function extraction around language.query, to use the
current tree-sitter Query and QueryCursor APIs and normalize dict-based captures
before processing nodes. Apply the same change across Go, Rust, Java, and Swift
extractors so their existing symbol extraction paths populate components instead
of being swallowed by broad exception handlers; otherwise remove those languages
from the documented support.
In `@docs/GRAPH_EXPORT_FEATURE.md`:
- Around line 21-22: Update the “Export as pickle” documentation around the wild
diff example and load_graph_from_pickle() guidance to state that pickle loading
can execute code and must only use files from trusted local producers. Recommend
JSON or GraphML when exchanging artifacts between users or systems.
In `@docs/PHASE1_SUMMARY.md`:
- Around line 106-108: Update the file-content resolution documentation and
implementation referenced by “File Content Handling” to use diff provenance:
select HEAD:<path> for staged content, :0:<path> for unstaged content, and the
working tree for new files, with appropriate handling for renamed files. Expand
content-fetch tests to cover staged, unstaged, and renamed files while
preserving the existing fallback behavior for untracked files.
In `@docs/TESTING_GUIDE.md`:
- Around line 9-11: Update docs/TESTING_GUIDE.md at lines 9-11, 24-26, 44-46,
121-124, 177-189, and 297-302 to use repository-root-relative paths:
tests/test_cli_manual.sh, tests/test_graph_export.py, and
tests/examples/example_usage.py as applicable. Update tests/test_cli_manual.sh
lines 101-106 so example_usage.py is resolved relative to the repository or
script directory rather than the caller’s working directory.
In `@README.md`:
- Around line 126-128: Update the shared graph-export call to use the selected
processor instead of agent.graph_manager, keeping all examples consistent:
README.md lines 126-128, docs/TESTING_GUIDE.md lines 38-40, and
docs/TESTING_GUIDE.md lines 115-118.
- Around line 121-124: Update the README tree-sitter example to use the existing
HTML adapter rather than invoking the incompatible direct CLI path. First align
the tree-sitter analysis output with the HTML generation contract by wiring a
schema v2 adapter or updating the relevant HTML-generation flow, then document
and test the command so it completes successfully without hanging.
In `@setup.py`:
- Line 9: Update the Python compatibility metadata in setup.py to match the
required tree-sitter-language-pack dependency: set python_requires to >=3.10 and
remove the Python 3.7–3.9 classifiers. Only remove the dependency instead if
tree-sitter mode is intentionally optional and the package can install without
it.
In `@tests/test_cli_manual.sh`:
- Around line 101-106: Update the Test 4 block around the example usage command
to capture python example_usage.py test_output.json output in a temporary file
before invoking head. Ensure the Python command runs independently under set -e
so its failure stops the script, then display the captured output with head and
preserve the existing success message only after both steps succeed.
- Around line 121-131: Update the cleanup flow in the manual harness so it never
blocks on read by default: remove the interactive read and delete the generated
test files automatically. If inspection is needed, support an explicit --keep
option that skips removal and reports the files were retained.
- Around line 46-49: Align schema examples and validation with v2: in
tests/test_cli_manual.sh lines 46-49, exercise structured JSON export or add a
separate v2 export case; in tests/test_cli_manual.sh lines 61-71, assert version
2.0 plus metadata, categorized files, source-code files/components, and
relationship edges; in docs/TESTING_GUIDE.md lines 225-256, replace the version
1.0 example or explicitly label it as legacy output.
- Around line 109-113: Update the Test 5 CLI help check in
tests/test_cli_manual.sh so missing --format or --graph-format options produce a
nonzero exit status instead of succeeding through the || branch. Preserve the
success message when grep finds the required options, and explicitly exit or
return failure after printing the error message.
In `@tests/test_graph_export.py`:
- Around line 46-47: Replace fixed working-directory export filenames with paths
created under a test-local temporary directory for each affected test. Apply
this to tests/test_graph_export.py lines 46-47, 69-70, and 84-85, and
tests/test_structured_export.py lines 118-119, 239-240, and 267-268, reusing the
test framework’s temporary-directory fixture and preserving each export format
and assertion flow.
- Around line 59-65: The graph export tests currently validate only node counts
or format markers instead of semantic round-trips. Extend the JSON and pickle
checks around load_graph_from_json and their corresponding loaders to compare
component edges, dependency/dependent lists, processed_files, file status,
summaries, and processed-file metadata against the original graph; load the
GraphML output with the NetworkX GraphML loader and perform equivalent semantic
comparisons rather than only checking XML/header content.
In `@tests/test_structured_export.py`:
- Around line 124-213: Update the structured export validation test to load the
v2 JSON Schema and call jsonschema.validate() on the parsed data before
field-level assertions, matching test_tree_sitter_phase1.py. Extend coverage
with canonical provenance, range, and rename fixtures or assertions so
validation checks schema semantics rather than only fixed counts. Preserve the
existing structural checks while ensuring all representative exports conform to
the schema contract.
---
Minor comments:
In `@CHANGELOG.md`:
- Around line 11-16: Update the “Tree-sitter Dependency Extraction (Phase 1)”
changelog entry to clearly distinguish Python as fully supported, while
describing TypeScript, JavaScript, Go, Rust, Java, and Swift as framework/parser
work in progress.
In `@diffgraph/processing_modes/tree_sitter_dependency.py`:
- Around line 746-761: The _extract_components dispatch must handle tsx and stop
using TypeScript queries for JavaScript. Pass the detected language through the
relevant extraction flow, route tsx appropriately, and update
_extract_components_typescript or its grammar-loading logic to use the matching
JavaScript, TypeScript, or TSX grammar so query node types align with the parser
returned by _get_parser.
In `@docs/GRAPH_EXPORT_FEATURE.md`:
- Around line 164-180: Update the Testing commands in GRAPH_EXPORT_FEATURE.md to
use repository-relative paths for tests/test_graph_export.py and
tests/examples/example_usage.py, while leaving the wild diff example unchanged.
In `@docs/PHASE1_SUMMARY.md`:
- Around line 186-190: Replace the hard-coded developer-specific path in the
“Priority 3: Testing & Validation” section with a portable repository-relative
example or an environment-variable-based location, while preserving the
instruction to use real repositories for validation.
- Around line 131-136: Update the extracted-components example in PHASE1_SUMMARY
so the expected output matches the sample input: remove another_function from
the “Output components extracted” list, unless the input is explicitly updated
to define it.
- Around line 82-95: The Tree-sitter documentation example in “Tree-sitter API
(v0.25+)” uses the wrong parser construction. Update it to demonstrate the same
ts.Parser-based initialization used by TreeSitterProcessor._get_parser(), and
align any nearby dependency/API guidance with that implementation instead of
tslp.get_parser(language).
- Line 154: Update the Python compatibility note in PHASE1_SUMMARY.md to remove
the claim that the project runs on Python 3.13, while retaining the
tree-sitter-language-pack requirement and only stating supported versions backed
by setup.py or project metadata.
- Around line 145-151: Pin the parser dependency to the exact tested parser and
binding versions in setup.py’s install_requires declaration, replacing the open
range. Update the corresponding dependency entry in CHANGELOG.md to use the same
pinned versions; the references in docs/PHASE1_SUMMARY.md and CHANGELOG.md are
documentation only and should match the final declaration.
---
Nitpick comments:
In `@diffgraph/processing_modes/__init__.py`:
- Around line 96-101: Update the optional processor import block in
diffgraph.processing_modes to catch only the missing tree-sitter-language-pack
dependency, log the skipped processor and installation hint through a module
logger, and allow other ImportError causes from tree_sitter_dependency.py
dependencies such as schema_v2_adapter or graph_manager to propagate. Add the
requested logging import and module-level logger, and correct the package name
in the skip message.
In `@diffgraph/processing_modes/schema_v2_adapter.py`:
- Around line 311-326: Update _build_file_entry and its callers to carry the
processor’s already-resolved language into each FileEntry, preserving null when
static analysis is unavailable. Extend the internal file-change data passed to
this function with lines_added and lines_removed from git diff metadata, and
include both fields in the emitted entry without performing another
language-detection pass.
In `@diffgraph/processing_modes/tree_sitter_dependency.py`:
- Around line 1036-1047: Remove the deprecated _analyze_changes_legacy method
and its dead-code dependencies, including GraphManager, ChangeType,
ComponentNode, _get_full_file_content, and _extract_function_calls. Also remove
the self.graph_manager and self.current_file_components assignments from
__init__, while preserving the active analyze_changes_v2()/analyze_changes()
paths.
- Around line 15-24: Remove the `tslp.ts = ts` module mutation in the import
shim, and re-raise the `ImportError` with exception chaining while preserving
the existing message. Update the query construction and cursor usage in the
relevant processing methods to call `ts.Query` and `ts.QueryCursor` directly
instead of `tslp.ts.Query` and `tslp.ts.QueryCursor`.
- Around line 816-831: Update _get_pre_change_content to remove the unused
staged parameter if HEAD is intentionally the base for both staged and unstaged
diffs, and revise its documentation and callers accordingly; otherwise, use
staged to select the correct Git revision, including :0: when required for
staged files.
In `@tests/test_schema_v2_adapter.py`:
- Around line 301-325: Update test_no_network_calls to accept pytest’s
monkeypatch fixture and use it to replace socket.socket.connect, preserving the
existing call recording and original-connect delegation. Remove the manual
assignment, original_connect restoration, and try/finally block while keeping
the assertion that _minimal_output() performs no connections.
In `@tests/test_tree_sitter_basic.py`:
- Around line 64-85: Update the import relationship verification in the test to
assert that the collected `import_rels` contains the expected `os` and `sys`
imports, rather than only printing them; keep the diagnostic output as needed.
Also replace the placeholder-free summary f-string with a normal string literal
to satisfy Ruff F541.
In `@tests/test_tree_sitter_phase1.py`:
- Around line 88-93: Remove the unused _make_processor_with_tmp_git helper, then
add an end-to-end test fixture that initializes git in tmp_path, commits
SIMPLE_PYTHON_FILE, replaces it with MODIFIED_PYTHON_FILE, and runs the
processor through the real pre/post lookup path. Assert the expected modified,
deleted, and unchanged change kinds and cover the parse-failure warning path
without relying on stubbed components.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 66020436-b723-45aa-a806-459de0297732
📒 Files selected for processing (17)
CHANGELOG.mdREADME.mddiffgraph/processing_modes/__init__.pydiffgraph/processing_modes/schema_v2_adapter.pydiffgraph/processing_modes/tree_sitter_dependency.pydiffgraph/schema/diffgraph-v2.schema.jsondocs/GRAPH_EXPORT_FEATURE.mddocs/PHASE1_SUMMARY.mddocs/TESTING_GUIDE.mdsetup.pytests/examples/example_usage.pytests/test_cli_manual.shtests/test_graph_export.pytests/test_schema_v2_adapter.pytests/test_structured_export.pytests/test_tree_sitter_basic.pytests/test_tree_sitter_phase1.py
🛑 Comments failed to post (9)
docs/GRAPH_EXPORT_FEATURE.md (1)
21-22: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Document the pickle trust boundary.
load_graph_from_pickle()usespickle.load(). A malicious pickle file can execute code during loading. State that users must load pickle exports only from trusted local producers. Recommend JSON or GraphML for exchanged artifacts.Also applies to: 137-143
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/GRAPH_EXPORT_FEATURE.md` around lines 21 - 22, Update the “Export as pickle” documentation around the wild diff example and load_graph_from_pickle() guidance to state that pickle loading can execute code and must only use files from trusted local producers. Recommend JSON or GraphML when exchanging artifacts between users or systems.docs/TESTING_GUIDE.md (1)
9-11: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use paths that resolve from the repository root.
The guide and harness use bare script names, but the scripts are under
tests/andtests/examples/.
docs/TESTING_GUIDE.md#L9-L11: calltests/test_cli_manual.sh.docs/TESTING_GUIDE.md#L24-L26: calltests/test_graph_export.py.docs/TESTING_GUIDE.md#L44-L46: calltests/examples/example_usage.py.docs/TESTING_GUIDE.md#L121-L124: use the sametests/examples/example_usage.pypath.docs/TESTING_GUIDE.md#L177-L189: update all verification commands.docs/TESTING_GUIDE.md#L297-L302: use the actual example-script path.tests/test_cli_manual.sh#L101-L106: resolveexample_usage.pyrelative to the repository or script directory.📍 Affects 2 files
docs/TESTING_GUIDE.md#L9-L11(this comment)docs/TESTING_GUIDE.md#L24-L26docs/TESTING_GUIDE.md#L44-L46docs/TESTING_GUIDE.md#L121-L124docs/TESTING_GUIDE.md#L177-L189docs/TESTING_GUIDE.md#L297-L302tests/test_cli_manual.sh#L101-L106🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/TESTING_GUIDE.md` around lines 9 - 11, Update docs/TESTING_GUIDE.md at lines 9-11, 24-26, 44-46, 121-124, 177-189, and 297-302 to use repository-root-relative paths: tests/test_cli_manual.sh, tests/test_graph_export.py, and tests/examples/example_usage.py as applicable. Update tests/test_cli_manual.sh lines 101-106 so example_usage.py is resolved relative to the repository or script directory rather than the caller’s working directory.tests/test_cli_manual.sh (4)
46-49: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Align documentation and tests with schema v2.
The current guide and harness describe or validate the legacy JSON shape. The v2 contract requires version
2.0, metadata, categorized files, source-code files/components, and relationship edges.
tests/test_cli_manual.sh#L46-L49: exercise structured JSON export or add a separate v2 export case.tests/test_cli_manual.sh#L61-L71: assert the v2 version and required nested fields.docs/TESTING_GUIDE.md#L225-L256: replace the version1.0example or label it as legacy output.📍 Affects 2 files
tests/test_cli_manual.sh#L46-L49(this comment)tests/test_cli_manual.sh#L61-L71docs/TESTING_GUIDE.md#L225-L256🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_cli_manual.sh` around lines 46 - 49, Align schema examples and validation with v2: in tests/test_cli_manual.sh lines 46-49, exercise structured JSON export or add a separate v2 export case; in tests/test_cli_manual.sh lines 61-71, assert version 2.0 plus metadata, categorized files, source-code files/components, and relationship edges; in docs/TESTING_GUIDE.md lines 225-256, replace the version 1.0 example or explicitly label it as legacy output.
101-106: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Preserve failures from the example command.
set -edoes not fail the pipeline whenpythonfails andheadsucceeds. The script can print “Example script works correctly” after a failed Python process. Capture the output first, then runheadon the captured file.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_cli_manual.sh` around lines 101 - 106, Update the Test 4 block around the example usage command to capture python example_usage.py test_output.json output in a temporary file before invoking head. Ensure the Python command runs independently under set -e so its failure stops the script, then display the captured output with head and preserve the existing success message only after both steps succeed.
109-113: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Exit when required CLI options are missing.
The
&& ... || ...expression prints an error but returns success after the||branch. The script can report all tests passed despite missing options.Proposed check
-python -m diffgraph.cli --help | grep -E "(--format|--graph-format)" && echo "✅ New CLI options are present" || echo "❌ CLI options missing" +if python -m diffgraph.cli --help | grep -qE "(--format|--graph-format)"; then + echo "✅ New CLI options are present" +else + echo "❌ CLI options missing" >&2 + exit 1 +fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.# Test 5: Test CLI help echo "Test 5: Verifying CLI options" echo "------------------------------" if python -m diffgraph.cli --help | grep -qE "(--format|--graph-format)"; then echo "✅ New CLI options are present" else echo "❌ CLI options missing" >&2 exit 1 fi echo ""🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_cli_manual.sh` around lines 109 - 113, Update the Test 5 CLI help check in tests/test_cli_manual.sh so missing --format or --graph-format options produce a nonzero exit status instead of succeeding through the || branch. Preserve the success message when grep finds the required options, and explicitly exit or return failure after printing the error message.
121-131: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Make the manual harness non-interactive by default.
read -pcan block automated or CI runs. The guide describes this script as the recommended automated test. Clean up by default, or add an explicit--keepoption for interactive inspection.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_cli_manual.sh` around lines 121 - 131, Update the cleanup flow in the manual harness so it never blocks on read by default: remove the interactive read and delete the generated test files automatically. If inspection is needed, support an explicit --keep option that skips removal and reports the files were retained.tests/test_graph_export.py (2)
46-47: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Isolate generated export artifacts in a temporary directory.
All sites write fixed filenames in the working directory. The export can overwrite an existing file before cleanup deletes it. Parallel test runs can also overwrite each other.
tests/test_graph_export.py#L46-L47: create the JSON path inside a test-local temporary directory.tests/test_graph_export.py#L69-L70: create the pickle path inside the same test-local temporary directory.tests/test_graph_export.py#L84-L85: create the GraphML path inside the same test-local temporary directory.tests/test_structured_export.py#L118-L119: create the structured-export path inside a test-local temporary directory.tests/test_structured_export.py#L239-L240: create the completeness-export path inside a test-local temporary directory.tests/test_structured_export.py#L267-L268: create the empty-graph-export path inside a test-local temporary directory.📍 Affects 2 files
tests/test_graph_export.py#L46-L47(this comment)tests/test_graph_export.py#L69-L70tests/test_graph_export.py#L84-L85tests/test_structured_export.py#L118-L119tests/test_structured_export.py#L239-L240tests/test_structured_export.py#L267-L268🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_graph_export.py` around lines 46 - 47, Replace fixed working-directory export filenames with paths created under a test-local temporary directory for each affected test. Apply this to tests/test_graph_export.py lines 46-47, 69-70, and 84-85, and tests/test_structured_export.py lines 118-119, 239-240, and 267-268, reusing the test framework’s temporary-directory fixture and preserving each export format and assertion flow.
59-65: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
For NetworkX 3.5, what API reads GraphML files, and what graph type does it return?💡 Result:
In NetworkX 3.5, the API used to read GraphML files is networkx.read_graphml [1]. Regarding the graph type returned: - The function returns a NetworkX graph object [1]. - A MultiGraph or MultiDiGraph is returned if the file contains parallel edges or if the force_multigraph parameter is set to True [1]. - Otherwise, a Graph or DiGraph is returned, depending on whether the file indicates the graph should be directed [1].
Citations:
🏁 Script executed:
#!/bin/bash set -euo pipefail printf 'Tracked test files:\n' git ls-files | rg '(^|/)test_graph_export\.py$|(^|/)test_structured_export\.py$' || true printf '\nOutline test_graph_export.py:\n' ast-grep outline tests/test_graph_export.py 2>/dev/null || true printf '\nRelevant contents:\n' cat -n tests/test_graph_export.py | sed -n '1,140p' printf '\nSearch graph export/export/import functions:\n' rg -n "def (load_graph_from_json|dump_graph_to_json|export_to_graphml|load_graphml|write_graphml|process_files|processed_files|dependency)" -S .Repository: WildestAI/DiffGraph-CLI
Length of output: 6125
🏁 Script executed:
#!/bin/bash set -euo pipefail printf 'Outline graph_export.py:\n' ast-grep outline diffgraph/graph_export.py 2>/dev/null || true printf '\nRelevant graph_export.py contents:\n' cat -n diffgraph/graph_export.py | sed -n '1,260p' printf '\nOutline graph_manager.py:\n' ast-grep outline diffgraph/graph_manager.py 2>/dev/null || true printf '\nGraphManager methods relevant to state:\n' rg -n "def (add_file|add_component|add_component_dependency|mark_processed|export_to_dict|file_nodes|component_nodes|file_graph|component_graph|processed_files|dependencies|get_)" diffgraph/graph_manager.py -A 60 -B 5Repository: WildestAI/DiffGraph-CLI
Length of output: 23980
Assert semantic round-trips for exported formats.
The graph includes component edges, dependency/dependent lists,
processed_files, file status, and summaries. The JSON, pickle, and GraphML paths currently assert only XML/header content or loaded node counts. Use the NetworkX GraphML loader with semantic comparison and add targeted assertions for JSON/pickle edges, processed files, status, summary, dependency lists, and processed-file metadata.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_graph_export.py` around lines 59 - 65, The graph export tests currently validate only node counts or format markers instead of semantic round-trips. Extend the JSON and pickle checks around load_graph_from_json and their corresponding loaders to compare component edges, dependency/dependent lists, processed_files, file status, summaries, and processed-file metadata against the original graph; load the GraphML output with the NetworkX GraphML loader and perform equivalent semantic comparisons rather than only checking XML/header content.tests/test_structured_export.py (1)
124-213: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail test -f diffgraph/schema/diffgraph-v2.schema.json rg -n -C 3 \ 'jsonschema|validate|diffgraph-v2\.schema|analyzed_at|old_path|diff_base|diff_target' \ tests/test_structured_export.py diffgraphRepository: WildestAI/DiffGraph-CLI
Length of output: 10776
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== test file outline/size ==" wc -l tests/test_structured_export.py ast-grep outline tests/test_structured_export.py || true echo "== test_structured_export relevant sections ==" sed -n '1,260p' tests/test_structured_export.py | nl -ba echo "== schema file top and relevant property names ==" python3 - <<'PY' import json from pathlib import Path p=Path("diffgraph/schema/diffgraph-v2.schema.json") data=json.loads(p.read_text()) print("schema_version", data.get("$schema"), data.get("properties",{}).keys(), data.get("required",[])) props=data.get("properties", {}) for k in ["metadata","auto_generated","documentation","configuration","source_code"]: print("\n---", k, "---") print("required", props.get(k,{}).get("required")) print("properties", list(props.get(k,{}).get("properties",{}).keys())) PY echo "== dependency and schema adapter references to validate imports ==" rg -n "import .*jsonschema|from .*jsonschema|validate\\(|analyze_changes_v2\\(|transform_to_structured_format|analyzed_at" \ tests diffgraph/processing_modes diffgraph/structured_export.pyRepository: WildestAI/DiffGraph-CLI
Length of output: 541
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== test_structured_export relevant sections ==" sed -n '1,260p' tests/test_structured_export.py echo "== schema top and key property names ==" python3 - <<'PY' import json from pathlib import Path p=Path("diffgraph/schema/diffgraph-v2.schema.json") data=json.loads(p.read_text()) print("schema_version", data.get("$schema"), data.get("properties",{}).keys(), data.get("required",[])) props=data.get("properties", {}) for k in ["metadata","auto_generated","documentation","configuration","source_code","components"]: s=props.get(k,{}) print("\n---", k, "---") print("type", s.get("type")) print("required", s.get("required")) print("properties", list(s.get("properties",{}).keys())) PY echo "== diffgraph references to schema validation and analyzed_at ==" rg -n "import .*jsonschema|from .*jsonschema|validate\\(|json\\.validate|analyze_changes_v2\\(|transform_to_structured_format|analyzed_at" \ tests diffgraph/processing_modes diffgraph/structured_export.py || true echo "== structured export output construction relevant sections ==" sed -n '180,235p' diffgraph/structured_export.py sed -n '312,430p' diffgraph/structured_export.pyRepository: WildestAI/DiffGraph-CLI
Length of output: 17739
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== tree_sitter schema validation example ==" sed -n '240,305p' tests/test_tree_sitter_phase1.py echo "== jsonschema availability ==" python3 - <<'PY' try: import jsonschema print("jsonschema", jsonschema.__version__, jsonschema.__file__) except Exception as exc: print("missing_or_error", type(exc).__name__, str(exc)) PY echo "== exact names in structured export schema files ==" python3 - <<'PY' import json, re from pathlib import Path files = ["diffgraph/schema/diffgraph-v2.schema.json", "diffgraph/processing_modes/schema_v2_adapter.py"] for p in files: print(f"\n--- {p} ---") text = Path(p).read_text() terms = ["analyzed_at", "generated_at", "wild_version", "diff_base", "diff_target", "total_files_changed", "source_code", "auto_generated", "documentation", "configuration", "version"] for term in terms: idx = text.find(term) print(f"{term}: {idx}") HYRepository: WildestAI/DiffGraph-CLI
Length of output: 3477
Validate structured export against the v2 JSON Schema contract.
The schema file contains the intended contract for exported fields such as
analyzed_at, andtest_tree_sitter_phase1.pyalready usesjsonschema.validate()for schema v2 output. Add the same validation here and cover canonical provenance, range, and rename cases so fixed counts do not mask schema drift.🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 124-124: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(output_path, 'r')
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').(open-filename-from-request)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_structured_export.py` around lines 124 - 213, Update the structured export validation test to load the v2 JSON Schema and call jsonschema.validate() on the parsed data before field-level assertions, matching test_tree_sitter_phase1.py. Extend coverage with canonical provenance, range, and rename fixtures or assertions so validation checks schema semantics rather than only fixed counts. Preserve the existing structural checks while ensuring all representative exports conform to the schema contract.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/test_tree_sitter_phase1.py (1)
1-45: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winFail CI when phase-one dependencies are missing.
tests/test_tree_sitter_phase1.pyskips all Tree-sitter tests intests/test_tree_sitter_phase1.py:40and schema validation tests intests/test_tree_sitter_phase1.py:305-307whentree-sitter-language-packorjsonschemais unavailable. Add these test constraints to the CI dependency/install step and require these tests to run there. Use skip markers only for a documented optional environment.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_tree_sitter_phase1.py` around lines 1 - 45, Update CI dependency installation to include tree-sitter-language-pack and jsonschema, then configure the CI test command to require tests in test_tree_sitter_phase1.py to execute rather than pass through skipif markers. Retain skip markers only for documented optional local environments, and ensure the schema validation and Tree-sitter acceptance tests fail CI when their dependencies are unavailable.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/test_tree_sitter_phase1.py`:
- Around line 194-212: Update test_post_parse_failure_is_reported to monkeypatch
_get_parser with a fake parser that initializes successfully but raises from
parse(), so the test directly exercises the post-change parsing failure path in
analyze_changes. Preserve the existing expected PARSE_FAILURE metadata and keep
parser initialization failure coverage in a separate test.
---
Outside diff comments:
In `@tests/test_tree_sitter_phase1.py`:
- Around line 1-45: Update CI dependency installation to include
tree-sitter-language-pack and jsonschema, then configure the CI test command to
require tests in test_tree_sitter_phase1.py to execute rather than pass through
skipif markers. Retain skip markers only for documented optional local
environments, and ensure the schema validation and Tree-sitter acceptance tests
fail CI when their dependencies are unavailable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 24aa706f-8a9d-4b95-bf41-54d25e8a59dc
📒 Files selected for processing (5)
diffgraph/processing_modes/schema_v2_adapter.pydiffgraph/processing_modes/tree_sitter_dependency.pysetup.pytests/test_schema_v2_adapter.pytests/test_tree_sitter_phase1.py
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/test_schema_v2_adapter.py
- diffgraph/processing_modes/schema_v2_adapter.py
- diffgraph/processing_modes/tree_sitter_dependency.py
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/test.yml (1)
16-18: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winPin GitHub Actions to immutable commits.
actions/checkout@v4andactions/setup-python@v5use mutable tags. Pin both actions to full commit SHAs and retain the release tag in a comment. This prevents an upstream tag change from altering the workflow without a repository change.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/test.yml around lines 16 - 18, Update the actions/checkout and actions/setup-python steps in the workflow to reference their full immutable commit SHAs instead of mutable version tags, while retaining each current release tag in an adjacent comment for readability.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/test.yml:
- Line 16: Update the actions/checkout@v4 step in the workflow to set
persist-credentials to false, unless a later job step explicitly requires
authenticated Git commands; leave the checkout behavior otherwise unchanged.
---
Nitpick comments:
In @.github/workflows/test.yml:
- Around line 16-18: Update the actions/checkout and actions/setup-python steps
in the workflow to reference their full immutable commit SHAs instead of mutable
version tags, while retaining each current release tag in an adjacent comment
for readability.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ab5168ff-49f8-4eba-b2e4-5b71e81b1e7f
📒 Files selected for processing (3)
.github/workflows/test.ymldiffgraph/cli.pytests/test_cli.py
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@requirements.txt`:
- Around line 4-5: Declare NetworkX in the authoritative package metadata used
by setup.py installation, adding the same Python-version split constraints shown
in requirements.txt to install_requires. Preserve the existing markers and
version bounds, and avoid relying solely on requirements.txt for the graph
dependency.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 419267ea-8786-4ddd-a187-1b5411c2f00e
📒 Files selected for processing (1)
requirements.txt
| networkx>=3.4.2,<3.5; python_version < "3.11" | ||
| networkx>=3.5; python_version >= "3.11" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 4 'install_requires|networkx|python_requires' setup.py requirements.txtRepository: WildestAI/DiffGraph-CLI
Length of output: 1200
🌐 Web query:
PyPA packaging install_requires differs from requirements.txt pip install package does not install requirements.txt dependencies documentation
💡 Result:
The confusion between install_requires and requirements.txt stems from their different roles in the Python packaging ecosystem. Pip does not automatically install dependencies listed in a requirements.txt file when you simply run pip install package_name, because requirements.txt files are not considered part of a package's metadata [1][2]. Key Differences: install_requires: This is defined in your project's configuration (e.g., pyproject.toml or setup.py) [3][2]. It specifies the abstract, minimal dependencies required for your package to function [1]. When someone installs your package via pip, pip reads this metadata and automatically installs these dependencies [3]. These requirements are "abstract" because they define what is needed but not where to get it from, leaving that to the installer's configuration [1]. requirements.txt: This file is a list of pip install arguments [1][2]. It is used to define a concrete, fully specified environment, often including pinned versions for all dependencies (including sub-dependencies) to ensure repeatable installations [1][2]. Pip only processes a requirements.txt file when you explicitly invoke it using the -r flag, such as: python -m pip install -r requirements.txt [1][4][5] Why Pip ignores requirements.txt: Pip determines a package's dependencies solely by analyzing the project's metadata (like install_requires in pyproject.toml or setup.py) [2][6]. It does not look for or automatically parse any requirements.txt files that might be included in a project's source code, as these files are intended for environment management rather than package metadata [1][2]. In summary, use install_requires to declare what your library needs to run, and use requirements.txt to lock down the exact versions of all packages in your development or production environment for reproducibility [1].
Citations:
- 1: https://packaging.python.org/en/latest/discussions/install-requires-vs-requirements/
- 2: https://pip.pypa.io/en/stable/user_guide/?highlight=requirements
- 3: https://setuptools.pypa.io/en/latest/userguide/dependency_management.html?highlight=install_requires
- 4: https://pip.pypa.io/en/stable/cli/pip_install.html
- 5: https://pip.pypa.io/en/latest/cli/pip%5Finstall/
- 6: https://pip.pypa.io/en/stable/user_guide/?highlight=
Declare NetworkX in package metadata.
requirements.txt does not drive package installation; installing this package through setup.py metadata only installs dependencies in install_requires. Since the graph imports require NetworkX, add the same split version constraints to setup.py or convert dependency declaration to an authoritative packaging source. The Python marker split is compatible because NetworkX 3.5 requires Python 3.11+.
Proposed metadata fix
install_requires=[
"click>=8.1.7",
+ 'networkx>=3.4.2,<3.5; python_version < "3.11"',
+ 'networkx>=3.5; python_version >= "3.11"',
"tree-sitter-language-pack>=0.10.0",
]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@requirements.txt` around lines 4 - 5, Declare NetworkX in the authoritative
package metadata used by setup.py installation, adding the same Python-version
split constraints shown in requirements.txt to install_requires. Preserve the
existing markers and version bounds, and avoid relying solely on
requirements.txt for the graph dependency.
Source: MCP tools
There was a problem hiding this comment.
Confirmed on current head 01d786f: requirements.txt declares the Python-version-split NetworkX constraints, but setup.py's install_requires omits NetworkX, so an installed package is not guaranteed to receive this required graph dependency. Please mirror the two markers in package metadata:
networkx>=3.4.2,<3.5; python_version < "3.11"networkx>=3.5; python_version >= "3.11"
I’m leaving the thread unresolved. I did not push to this contributor-owned branch because GitHub reports maintainerCanModify=false.
There was a problem hiding this comment.
@nia-sg-bot, acknowledged. The finding remains valid.
setup.py must declare the same two NetworkX constraints in install_requires. This ensures that pip install installs NetworkX for supported Python versions. The thread should remain unresolved until that change is present.
You are interacting with an AI system.
nia-sg-bot
left a comment
nia-sg-bot
left a comment
There was a problem hiding this comment.
CI on current head 01d786f is failing in tests/test_cli.py::test_cli_invalid_mode_error (run 30812052374): wild diff --mode invalid-mode exits 0 with “No changes to analyze” when the checkout has no changes. The current flow returns before get_processor() validates mode (diffgraph/cli.py validates only after changed-file discovery), so invalid user input can be reported as success depending on repository state.
Please validate the selected mode before the no-changes early return and make the test deterministic with an explicit no-change repository fixture (or otherwise assert invalid-mode rejection independently of ambient changes). This is direction-aligned CLI correctness and removes the current red gate. The extension and website contain no matching fixture or released claim for these messages, so no cross-repo update is indicated.
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai After your last review, I don't see any new comments added by you. Do you approve this PR or are you still not satisfied? |
|
Tip For best results, initiate chat on the files or code changes.
The last review runs were incremental. They did not find new changes to comment on. They do not replace the product-direction verdict. The current verdict remains REVISE. Before approval, the PR needs the requested deterministic-subset rebase or rebuild on current After those changes are present, request a new review. You are interacting with an AI system. |
|
The useful deterministic work from this stacked PR has now been migrated into #24, which targets current #24 preserves the exact Git snapshot resolution, local Python Tree-sitter extraction, validated schema-v2 artifact, stable structural IDs/order, evidence, scoped warnings, and network-free CLI path. It intentionally does not carry forward the stacked legacy exporters, GraphManager/Mermaid canonicalization, AI-default processing registry, divergent schema copy, or unrelated stale release/docs history. I’m leaving #13 open for the moment so the replacement can be reviewed and its provenance remains clear; once #24 is accepted as the replacement, #13 can be closed as superseded. |
|
Closing as superseded by #24, the clean replacement based directly on current main. The original discussion and full stacked diff remain available here for provenance. |
2 participants
Phase 1: Tree-sitter Dependency Extraction + Schema V2 Output
This PR implements the tree-sitter AST-based dependency extraction processor and wires it to produce schema v2 output — the foundational layer for DiffGraph v2.
What's included
Phase 1 acceptance criteria (all met)
TreeSitterProcessor.analyze_changes()returns schema v2 dict (not GraphManager)privacy_tier == "local"diffgraph/schema/diffgraph-v2.schema.jsonrelationships[]includes import relationships for Python filesmetadata.analysis_duration_msis presentKey fixes (from latest commits)
_get_parser: was using incompatible native API; now usests.Parser(tslp.get_language())build_symbol_entry: added required schema v2 fields (name, file_id, kind, parent_id)build_import_relationship: renamed from/to → source_id/target_id; added id fieldbuild_schema_v2_output: fixed diff_ref schema violations, moved warnings into metadataWhat this enables
Once merged:
wild diffcan emit schema v2 JSON (structural tier) from local static analysis — no API call neededMerge sequence context
This is Phase 1 in the v2 implementation roadmap. After this merges:
→ Phase 2 fixes go on
feature/multiple-processing-modes(PR #12)→ Phase 3 fixes go on
feature/output-graph-data(PR #11)→ Phase 4 is a new branch (terminal formatter)
See
docs/DiffGraph-CLI/design/V2-IMPLEMENTATION-ROADMAP.mdfor full plan.Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
Compatibility