Feature: output graph data as JSON#11
Open
avikalpg wants to merge 5 commits into
Open
Conversation
Add ability to export complete networkx graph data structure to file formats that other programs can consume programmatically. New Features: - Add --format option to choose between 'html' (default) and 'graph' output - Add --graph-format option to select serialization (json/pickle/graphml) - Support for JSON export (human-readable, widely compatible) - Support for Pickle export (Python-native, exact data preservation) - Support for GraphML export (standard graph format for analysis tools) New Files: - diffgraph/graph_export.py: Core export/import functionality - test_graph_export.py: Comprehensive test suite - example_usage.py: Example script for using exported data - test_cli_manual.sh: Automated testing script - GRAPH_EXPORT_FEATURE.md: Feature documentation - TESTING_GUIDE.md: Testing instructions Modified Files: - diffgraph/cli.py: Added new CLI options and conditional output logic - diffgraph/graph_manager.py: Added export_to_dict() method - README.md: Updated documentation with new features - CHANGELOG.md: Added v1.1.0 release notes - setup.py: Bumped version to 1.1.0 - diffgraph/__init__.py: Updated version to 1.1.0 Technical Details: - Exported data includes file nodes, component nodes, graphs, and metadata - All formats support round-trip (export → load) with data integrity - NetworkX graphs serialized using node-link format for compatibility - Backward compatible: existing HTML functionality unchanged Testing: - All unit tests pass (test_graph_export.py) - Automated test suite validates all formats - Example usage script demonstrates practical use cases
Add comprehensive design documents for new integration-friendly JSON output format to docs/planning/ folder to keep root directory clean. Documents: - STRUCTURED_OUTPUT_DESIGN.md: Complete schema specification - File categorization (source/docs/config/auto-generated) - Rich component metadata (complexity, impact radius) - External dependency nodes (APIs, databases, services) - Comprehensive relationship types (REST, RPC, pub/sub) - Cross-references between docs and code - Phase-based implementation strategy - PHASE1_IMPLEMENTATION_NOTES.md: Implementation guide - Implementation decisions and rationale - Phase 1 scope (basic restructuring with existing data) - Phase 2/3 future enhancements - Testing strategy and success criteria - Next session pickup instructions These documents preserve design rationale and guide future implementation without cluttering the codebase root.
Add integration-friendly structured JSON format optimized for VSCode extensions, web UIs, and other tool integrations. This is Phase 1 using existing analysis data without requiring new AI analysis. New Features: - Structured JSON output with clean categorization and rich metadata - Automatic file classification into 4 categories: * auto_generated: Lock files, build artifacts (excluded from review) * documentation: Docs with potential cross-references to code * configuration: Config files with change tracking * source_code: Source files with full dependency graphs - Git diff stats (additions/deletions) extracted per file - Impact radius calculation from dependency graph - Complete graph structure (all edge targets exist as nodes) - Pattern-based classification with 40+ common file patterns New Files: - diffgraph/structured_export.py: Core transformation logic (400+ lines) * File categorization and classification * Component and edge transformation * Git stats extraction * Structured format generation - test_structured_export.py: Comprehensive test suite * File classification tests * Component transformation tests * Full export validation * Graph completeness tests * Empty graph handling Modified Files: - diffgraph/cli.py: Route JSON format to structured export - README.md: Document structured format with examples - CHANGELOG.md: Added v1.2.0 release notes - setup.py: Bumped version to 1.2.0 - diffgraph/__init__.py: Updated version to 1.2.0 Breaking Changes: - JSON format now outputs structured format (v2.0 schema) - NetworkX format still available via pickle/graphml Technical Details: - Phase 1: Uses existing data, leaves advanced fields as null - Relationship types: imports, calls, extends, implements - Change types: added, deleted, modified, unchanged - Clean separation of files graph and components graph - All tests pass (5/5 test suites) Phase 2 Planned: - Full codebase impact analysis - External dependency nodes (APIs, databases) - Advanced relationships (REST, RPC, pub/sub) - Complexity metrics and line numbers - Cross-reference detection See docs/planning/STRUCTURED_OUTPUT_DESIGN.md for complete specification.
…id', but just 'parent'. refactor: Clean up whitespace in structured_export.py This commit removes unnecessary whitespace throughout the structured_export.py file, improving code readability without altering functionality. The changes include adjustments to spacing in function definitions, docstrings, and return statements, ensuring a more consistent coding style.
Contributor
WalkthroughThis PR adds DiffGraph v2 structured JSON export, legacy JSON/Pickle/GraphML serialization, CLI format routing, a strict JSON Schema, graph export tests, and release and usage documentation. ChangesStructured DiffGraph export
Legacy graph serialization
CLI integration
Supporting updates
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant GraphManager
participant StructuredExport
participant FileSystem
User->>CLI: select JSON output format
CLI->>GraphManager: analyze diff
CLI->>StructuredExport: export_diffgraph_v2
StructuredExport->>GraphManager: read nodes and graph edges
StructuredExport->>StructuredExport: build and validate artifact
StructuredExport->>FileSystem: write JSON
FileSystem-->>User: return output path
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
diffgraph/graph_export.py (1)
109-166: Code duplication noted.This function duplicates the logic in
GraphManager.export_to_dict()(lines 305-360 in graph_manager.py). See the comment on that file for refactoring suggestions.
🧹 Nitpick comments (6)
CHANGELOG.md (1)
8-36: Well-documented release notes!The changelog clearly documents the Phase 1 structured JSON output format and breaking changes. The phased approach and technical details provide good context for users.
Optional style improvement: Consider hyphenating "backwards-compatible" on line 23 per standard English compound adjective usage.
- - **JSON format now uses structured output by default** (breaking change for JSON, but backwards compatible overall) + - **JSON format now uses structured output by default** (breaking change for JSON, but backwards-compatible overall)docs/planning/STRUCTURED_OUTPUT_DESIGN.md (1)
1-643: Excellent design document!This comprehensive design document provides a clear roadmap for the structured JSON output format, with well-defined phases and implementation strategy. The distinction between Phase 1 (basic restructuring) and Phase 2 (enhanced analysis) sets realistic expectations.
Optional markdown improvements for better rendering:
- Add language specifier to the code block at line 93
- Remove spaces inside emphasis markers (lines 126, 155)
These are minor formatting issues that don't affect the excellent content.
test_graph_export.py (1)
1-1: Shebang without executable permission.The file has a shebang line but is not marked as executable. Either make the file executable or remove the shebang since this is typically imported as a module.
# Option 1: Make it executable chmod +x test_graph_export.py # Option 2: Remove the shebang if not neededAs per static analysis.
diffgraph/graph_manager.py (1)
324-331: Simplify component serialization logic.The component list comprehension uses nested conditionals with
hasattrchecks that may not be necessary. TheFileNode.componentsfield is typed asList[Dict], suggesting components should already be dictionaries.If
componentsare always dictionaries (as the type hint suggests), simplify to:-'components': [ - { - 'name': c.name if hasattr(c, 'name') else str(c), - 'change_type': c.change_type if hasattr(c, 'change_type') else 'unknown', - 'summary': c.summary if hasattr(c, 'summary') else None - } if hasattr(c, '__dict__') else str(c) - for c in (node.components or []) -] +'components': node.components or []If components can be mixed types, add a comment explaining why and consider validating the type earlier in the flow.
example_usage.py (1)
47-47: Rename unused loop variable.The loop variable
component_idis not used in the loop body.Apply this diff:
- for component_id, component_node in graph_manager.component_nodes.items(): + for _, component_node in graph_manager.component_nodes.items():Similar issue at line 64.
docs/planning/PHASE1_IMPLEMENTATION_NOTES.md (1)
300-314: Add language identifier to fenced code block.The commit message template code block (line 300) is missing a language identifier.
Apply this diff:
-``` +```text feat: Add structured JSON output format (Phase 1) ...</blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used**: CodeRabbit UI **Review profile**: CHILL **Plan**: Pro <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between a43abac1e9bb30104aab181cc68e1b306abe4a70 and d43d668f05e8b913c665d7b312c000c4e9875033. </details> <details> <summary>📒 Files selected for processing (16)</summary> * `CHANGELOG.md` (1 hunks) * `GRAPH_EXPORT_FEATURE.md` (1 hunks) * `README.md` (2 hunks) * `TESTING_GUIDE.md` (1 hunks) * `diffgraph/__init__.py` (1 hunks) * `diffgraph/cli.py` (3 hunks) * `diffgraph/graph_export.py` (1 hunks) * `diffgraph/graph_manager.py` (1 hunks) * `diffgraph/structured_export.py` (1 hunks) * `docs/planning/PHASE1_IMPLEMENTATION_NOTES.md` (1 hunks) * `docs/planning/STRUCTURED_OUTPUT_DESIGN.md` (1 hunks) * `example_usage.py` (1 hunks) * `setup.py` (1 hunks) * `test_cli_manual.sh` (1 hunks) * `test_graph_export.py` (1 hunks) * `test_structured_export.py` (1 hunks) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🧬 Code graph analysis (6)</summary> <details> <summary>test_graph_export.py (2)</summary><blockquote> <details> <summary>diffgraph/graph_manager.py (8)</summary> * `GraphManager` (54-360) * `ChangeType` (8-13) * `FileStatus` (15-20) * `add_file` (85-94) * `add_component` (96-122) * `add_component_dependency` (124-135) * `mark_processed` (150-156) * `export_to_dict` (305-360) </details> <details> <summary>diffgraph/graph_export.py (3)</summary> * `export_graph` (249-273) * `load_graph_from_json` (169-182) * `load_graph_from_pickle` (185-198) </details> </blockquote></details> <details> <summary>example_usage.py (1)</summary><blockquote> <details> <summary>diffgraph/graph_export.py (1)</summary> * `load_graph_from_json` (169-182) </details> </blockquote></details> <details> <summary>diffgraph/cli.py (2)</summary><blockquote> <details> <summary>diffgraph/graph_export.py (1)</summary> * `export_graph` (249-273) </details> <details> <summary>diffgraph/structured_export.py (1)</summary> * `export_structured_json` (403-433) </details> </blockquote></details> <details> <summary>diffgraph/graph_export.py (1)</summary><blockquote> <details> <summary>diffgraph/graph_manager.py (5)</summary> * `GraphManager` (54-360) * `FileNode` (23-34) * `ComponentNode` (37-52) * `ChangeType` (8-13) * `FileStatus` (15-20) </details> </blockquote></details> <details> <summary>test_structured_export.py (2)</summary><blockquote> <details> <summary>diffgraph/graph_manager.py (6)</summary> * `GraphManager` (54-360) * `ChangeType` (8-13) * `add_file` (85-94) * `add_component` (96-122) * `add_component_dependency` (124-135) * `mark_processed` (150-156) </details> <details> <summary>diffgraph/structured_export.py (5)</summary> * `classify_file` (53-75) * `export_structured_json` (403-433) * `transform_component_node` (175-202) * `transform_file_node` (205-224) * `transform_to_structured_format` (312-400) </details> </blockquote></details> <details> <summary>diffgraph/structured_export.py (1)</summary><blockquote> <details> <summary>diffgraph/graph_manager.py (4)</summary> * `GraphManager` (54-360) * `FileNode` (23-34) * `ComponentNode` (37-52) * `ChangeType` (8-13) </details> </blockquote></details> </details><details> <summary>🪛 LanguageTool</summary> <details> <summary>CHANGELOG.md</summary> [uncategorized] ~23-~23: If this is a compound adjective that modifies the following noun, use a hyphen. Context: ...efault** (breaking change for JSON, but backwards compatible overall) - `--format graph --graph-fo... (EN_COMPOUND_ADJECTIVE_INTERNAL) </details> </details> <details> <summary>🪛 markdownlint-cli2 (0.18.1)</summary> <details> <summary>docs/planning/PHASE1_IMPLEMENTATION_NOTES.md</summary> 300-300: Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> <details> <summary>docs/planning/STRUCTURED_OUTPUT_DESIGN.md</summary> 93-93: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 126-126: Spaces inside emphasis markers (MD037, no-space-in-emphasis) --- 155-155: Spaces inside emphasis markers (MD037, no-space-in-emphasis) </details> </details> <details> <summary>🪛 Ruff (0.14.2)</summary> <details> <summary>test_graph_export.py</summary> 1-1: Shebang is present but file is not executable (EXE001) </details> <details> <summary>example_usage.py</summary> 1-1: Shebang is present but file is not executable (EXE001) --- 47-47: Loop control variable `component_id` not used within loop body Rename unused `component_id` to `_component_id` (B007) --- 64-64: Loop control variable `component_id` not used within loop body Rename unused `component_id` to `_component_id` (B007) </details> <details> <summary>diffgraph/cli.py</summary> 238-238: `subprocess` call: check for execution of untrusted input (S603) --- 238-238: Starting a process with a partial executable path (S607) --- 240-240: Starting a process without a shell (S606) --- 242-242: `subprocess` call: check for execution of untrusted input (S603) --- 242-242: Starting a process with a partial executable path (S607) </details> <details> <summary>diffgraph/graph_export.py</summary> 196-196: `pickle` and modules that wrap it can be unsafe when used to deserialize untrusted data, possible security issue (S301) --- 273-273: Avoid specifying long messages outside the exception class (TRY003) </details> <details> <summary>test_structured_export.py</summary> 1-1: Shebang is present but file is not executable (EXE001) </details> <details> <summary>diffgraph/structured_export.py</summary> 90-90: Consider iterable unpacking instead of concatenation Replace with iterable unpacking (RUF005) --- 91-91: `subprocess` call: check for execution of untrusted input (S603) --- 100-100: Do not catch blind exception: `Exception` (BLE001) --- 177-177: Unused function argument: `graph_manager` (ARG001) --- 248-248: Unused function argument: `graph_manager` (ARG001) --- 405-405: PEP 484 prohibits implicit `Optional` Convert to `T | None` (RUF013) </details> </details> </details> <details> <summary>🔇 Additional comments (11)</summary><blockquote> <details> <summary>setup.py (1)</summary><blockquote> `5-5`: **LGTM!** The version bump to 1.2.0 appropriately reflects the new graph export features being added in this PR. </blockquote></details> <details> <summary>TESTING_GUIDE.md (1)</summary><blockquote> `1-302`: **Excellent testing documentation!** This comprehensive testing guide provides clear instructions for verifying the new graph export feature across multiple scenarios. The step-by-step examples and troubleshooting section will be valuable for both users and contributors. </blockquote></details> <details> <summary>GRAPH_EXPORT_FEATURE.md (1)</summary><blockquote> `1-187`: **Comprehensive feature documentation!** This document provides excellent coverage of the graph export feature, including clear usage examples, data structure descriptions, and implementation details. The backward compatibility note is particularly helpful. </blockquote></details> <details> <summary>diffgraph/__init__.py (1)</summary><blockquote> `5-5`: **LGTM!** Version bump to 1.2.0 is consistent with setup.py and appropriately reflects the new features. </blockquote></details> <details> <summary>test_graph_export.py (1)</summary><blockquote> `13-111`: **Well-structured test coverage!** The test function comprehensively validates all export formats (JSON, pickle, GraphML) with proper assertions for data integrity and file creation. The cleanup step is a good practice. </blockquote></details> <details> <summary>README.md (2)</summary><blockquote> `9-79`: **Clear documentation of new features!** The updated usage section effectively communicates the new graph export capabilities with clear examples for all supported formats (JSON, Pickle, GraphML). --- `81-184`: **Excellent output format documentation!** The new "Output Formats" section provides comprehensive coverage of both HTML reports and graph data export. The structured JSON format explanation with categorization and code examples is particularly helpful for users integrating with the tool. </blockquote></details> <details> <summary>diffgraph/cli.py (1)</summary><blockquote> `211-242`: **LGTM! Clean format-based routing.** The implementation correctly branches between graph export and HTML report generation based on the `--format` flag. The use of structured JSON for `--graph-format json` and NetworkX format for pickle/graphml is well-aligned with the Phase 1 design. Note: The static analysis warnings about subprocess calls (lines 238, 240, 242) are false positives—these commands open the browser with system utilities and don't execute untrusted input. </blockquote></details> <details> <summary>test_structured_export.py (1)</summary><blockquote> `19-284`: **Excellent test coverage for Phase 1!** The test suite comprehensively validates: - File classification across all categories (auto-generated, docs, config, source) - Component transformation with Phase 2 null fields - End-to-end structured export with proper categorization - Graph completeness (all edges reference valid nodes) - Empty graph handling This provides strong validation of the Phase 1 structured export implementation. Minor: Consider making the file executable since it has a shebang (line 1). </blockquote></details> <details> <summary>diffgraph/graph_export.py (1)</summary><blockquote> `185-198`: **Document pickle security considerations.** Pickle can be unsafe when deserializing untrusted data. Since this function loads graph data that may come from external sources, consider documenting the security implications. Add a security warning to the docstring: ```diff def load_graph_from_pickle(pickle_path: str) -> GraphManager: """ Load graph data from a pickle file. + + ⚠️ Security Warning: Only load pickle files from trusted sources. + Pickle can execute arbitrary code during deserialization. Args: pickle_path: Path to the pickle fileAlternatively, if pickle files are only used internally or in trusted environments, document that assumption.
diffgraph/structured_export.py (1)
175-202: Unused parameter is acceptable for Phase 1.The
graph_managerparameter is currently unused but is likely needed for Phase 2 enhancements (relationship type detection based on component analysis). Keeping it maintains a consistent API.Similar unused parameter at line 248 in
transform_file_edgeis also acceptable for Phase 2 extensibility.
Comment on lines
+305
to
+360
| def export_to_dict(self) -> dict: | ||
| """ | ||
| Export the graph manager state to a dictionary. | ||
| This is useful for serialization and can be passed to graph_export functions. | ||
|
|
||
| Returns: | ||
| Dictionary containing all graph data and metadata | ||
| """ | ||
| from networkx.readwrite import json_graph | ||
|
|
||
| # Convert file nodes to serializable format | ||
| file_nodes_data = {} | ||
| for file_path, node in self.file_nodes.items(): | ||
| file_nodes_data[file_path] = { | ||
| 'path': node.path, | ||
| 'status': node.status.value, | ||
| 'change_type': node.change_type.value, | ||
| 'summary': node.summary, | ||
| 'error': node.error, | ||
| 'components': [ | ||
| { | ||
| 'name': c.name if hasattr(c, 'name') else str(c), | ||
| 'change_type': c.change_type if hasattr(c, 'change_type') else 'unknown', | ||
| 'summary': c.summary if hasattr(c, 'summary') else None | ||
| } if hasattr(c, '__dict__') else str(c) | ||
| for c in (node.components or []) | ||
| ] | ||
| } | ||
|
|
||
| # Convert component nodes to serializable format | ||
| component_nodes_data = {} | ||
| for component_id, node in self.component_nodes.items(): | ||
| component_nodes_data[component_id] = { | ||
| 'name': node.name, | ||
| 'file_path': node.file_path, | ||
| 'change_type': node.change_type.value, | ||
| 'component_type': node.component_type, | ||
| 'parent': node.parent, | ||
| 'summary': node.summary, | ||
| 'dependencies': node.dependencies, | ||
| 'dependents': node.dependents | ||
| } | ||
|
|
||
| # Convert graphs to node-link format | ||
| file_graph_data = json_graph.node_link_data(self.file_graph, edges="links") | ||
| component_graph_data = json_graph.node_link_data(self.component_graph, edges="links") | ||
|
|
||
| # Combine all data | ||
| return { | ||
| 'version': '1.0', | ||
| 'file_nodes': file_nodes_data, | ||
| 'component_nodes': component_nodes_data, | ||
| 'file_graph': file_graph_data, | ||
| 'component_graph': component_graph_data, | ||
| 'processed_files': list(self.processed_files) | ||
| } No newline at end of file |
Contributor
There was a problem hiding this comment.
Code duplication with graph_export.py.
The export_to_dict() method duplicates the serialization logic in diffgraph/graph_export.py's _prepare_graph_data() function (lines 109-166). Both functions perform identical transformations of file_nodes, component_nodes, and graphs.
Consider one of these approaches:
Option 1 (Recommended): Make _prepare_graph_data call export_to_dict
# In diffgraph/graph_export.py
def _prepare_graph_data(graph_manager: GraphManager) -> Dict[str, Any]:
- # Convert file nodes to serializable format
- file_nodes_data = {}
- for file_path, node in graph_manager.file_nodes.items():
- ...
- # ... rest of duplication
- return data
+ """Prepare graph data for serialization by delegating to GraphManager."""
+ return graph_manager.export_to_dict()Option 2: Remove export_to_dict and keep logic in graph_export.py
If export_to_dict isn't used elsewhere, consider removing it entirely and keeping serialization logic centralized in the graph_export module.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In diffgraph/graph_manager.py around lines 305-360, export_to_dict duplicates
the serialization logic already implemented in
diffgraph/graph_export.py::_prepare_graph_data (lines ~109-166); replace the
duplicated implementation by delegating to that centralized function (import
_prepare_graph_data from diffgraph.graph_export and return its result) or, if
export_to_dict is unused elsewhere, remove this method and update callers to use
graph_export._prepare_graph_data directly; ensure the function signatures match,
import paths are correct, update any unit tests or call sites, and keep returned
structure/version identical to avoid breaking callers.
| 'total_files_changed': total_files, | ||
| 'total_additions': total_additions, | ||
| 'total_deletions': total_deletions, | ||
| 'analyzer_version': '1.2.0' # TODO: Get from package |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Replace TODO with actual version.
The analyzer version is hardcoded with a TODO comment.
Import the version from the package:
+from . import __version__
+
# ...
'metadata': {
'analyzed_at': datetime.now(timezone.utc).isoformat(),
'diff_base': diff_base,
'diff_target': diff_target,
'total_files_changed': total_files,
'total_additions': total_additions,
'total_deletions': total_deletions,
- 'analyzer_version': '1.2.0' # TODO: Get from package
+ 'analyzer_version': __version__
},📝 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.
Suggested change
| 'analyzer_version': '1.2.0' # TODO: Get from package | |
| from . import __version__ | |
| # ... other code ... | |
| 'metadata': { | |
| 'analyzed_at': datetime.now(timezone.utc).isoformat(), | |
| 'diff_base': diff_base, | |
| 'diff_target': diff_target, | |
| 'total_files_changed': total_files, | |
| 'total_additions': total_additions, | |
| 'total_deletions': total_deletions, | |
| 'analyzer_version': __version__ | |
| }, |
🤖 Prompt for AI Agents
In diffgraph/structured_export.py around line 394, the analyzer_version is
hardcoded as '1.2.0' with a TODO; replace the hardcoded value by importing the
package version (e.g., from your package's __init__ or version module) and use
that imported variable for 'analyzer_version'. Add the appropriate import at the
top of the file, handle import errors if needed (fallback or raise), and remove
the TODO comment.
| @@ -0,0 +1,88 @@ | |||
| #!/usr/bin/env python | |||
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Make the script executable.
The shebang is present, but the file lacks executable permissions.
Run:
chmod +x example_usage.py🧰 Tools
🪛 Ruff (0.14.2)
1-1: Shebang is present but file is not executable
(EXE001)
🤖 Prompt for AI Agents
In example_usage.py at line 1, the file has a shebang but lacks executable
permission; make it executable by setting the POSIX execute bit for the intended
users (e.g., run chmod +x example_usage.py) so the script can be run directly
from the shell while retaining current owner/group and other permissions.
| @@ -0,0 +1,145 @@ | |||
| #!/bin/bash | |||
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Make the script executable.
The shebang is present, but the file lacks executable permissions.
Run:
chmod +x test_cli_manual.sh🤖 Prompt for AI Agents
In test_cli_manual.sh around line 1, the script has a shebang but lacks
executable permissions; fix by setting the file mode to include the execute bit
for the appropriate user(s) (e.g., give the owner execute permission) so the
script can be run directly.
4 tasks
This was referenced Jul 8, 2026
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.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
diffgraph/cli.py (1)
138-144: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAvoid shadowing the Python built-in
format.The CLI argument name
formatshadows a Python built-in function. While common in CLI scripts, it can be easily avoided by mapping an explicit variable name (output_format) via the Click option and updating its references throughout the function.
diffgraph/cli.py#L138-L144: Update the click option to@click.option('--format', '-f', 'output_format', ...)and the function signature to useoutput_format: str.diffgraph/cli.py#L147-L149: Update the default output path condition to checkif output_format == 'json':.diffgraph/cli.py#L208-L216: Update the export routing condition to checkif output_format == '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 `@diffgraph/cli.py` around lines 138 - 144, Rename the Click-provided parameter from format to output_format in main while retaining the --format/-f CLI flags. Update diffgraph/cli.py lines 138-144 to declare the mapped parameter and signature, and change the output-path condition at lines 147-149 and export-routing condition at lines 208-216 to use output_format.Source: Linters/SAST tools
🤖 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/schema/diffgraph-v2.schema.json`:
- Around line 101-139: Update the Evidence schema to reject unknown properties
with additionalProperties: false, then add kind-based conditional validation
requiring the appropriate payload fields for each enum value, including model
and prompt_ref for llm_inference and the relevant file, location, pattern,
detail, or structural-basis fields for other kinds. Ensure kind-only evidence
objects no longer validate.
- Around line 177-180: Update the FileEntry change_kind schema definition to
include "unchanged" in its enum, matching the value emitted by
structured_export.py for ChangeType.UNCHANGED. Preserve all existing enum
values.
In `@diffgraph/structured_export.py`:
- Around line 30-68: The _diff_ref_from_args function must preserve the `--`
separator, parse only arguments before it as revisions, and treat arguments
after it exclusively as pathspecs. Update commit-range detection so refs such as
`feature/foo` are not classified as paths, and emit the parsed pathspecs through
the expected `file_scope` field. Keep staged and unstaged handling intact while
ensuring `main -- src/app.py` represents a single revision plus the file scope,
not a commit range.
- Around line 218-225: Update both relationship builders in structured export to
include a numeric confidence value whenever analysis_source is "inferred". Add
the required confidence field to the entries constructed in the shown imports
relationship block and the corresponding second builder, using the established
inference-confidence value if one exists.
- Around line 174-184: Update the kind_map used by the structured export
normalization to emit only schema-accepted SymbolEntry.kind values: function,
class, method, import, constant, type_alias, or module. Replace the unsupported
interface, variable, and other outputs with appropriate schema kinds, and
explicitly decide how container components should map rather than relying on the
invalid fallback.
- Around line 359-377: The validation branch in the structured export flow must
prevent writing invalid artifacts when validate is true. Update the
jsonschema.ValidationError handler around jsonschema.validate to re-raise the
exception or return before the output-writing block; preserve successful
validation and missing-jsonschema behavior.
In `@tests/test_diffgraph_v2_export.py`:
- Around line 327-328: Add jsonschema to the project’s test dependency
configuration so the dependency is installed in the test environment. Ensure
both schema-validation tests, including test_output_validates_against_schema,
run instead of being skipped by pytest.importorskip('jsonschema').
---
Nitpick comments:
In `@diffgraph/cli.py`:
- Around line 138-144: Rename the Click-provided parameter from format to
output_format in main while retaining the --format/-f CLI flags. Update
diffgraph/cli.py lines 138-144 to declare the mapped parameter and signature,
and change the output-path condition at lines 147-149 and export-routing
condition at lines 208-216 to use output_format.
🪄 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
Run ID: cb3d705f-907e-49cb-bc3b-854941320ed6
📒 Files selected for processing (5)
diffgraph/cli.pydiffgraph/graph_export.pydiffgraph/schema/diffgraph-v2.schema.jsondiffgraph/structured_export.pytests/test_diffgraph_v2_export.py
🚧 Files skipped from review as they are similar to previous changes (1)
- diffgraph/graph_export.py
Comment on lines
+101
to
+139
| "Evidence": { | ||
| "type": "object", | ||
| "description": "Pointer to what produced a claim. kind determines which fields are present.", | ||
| "required": ["kind"], | ||
| "properties": { | ||
| "kind": { | ||
| "type": "string", | ||
| "enum": [ | ||
| "git_diff_stat", | ||
| "git_diff_name_status", | ||
| "path_pattern", | ||
| "ast_parse", | ||
| "import_statement", | ||
| "call_site", | ||
| "llm_inference", | ||
| "structural_basis" | ||
| ] | ||
| }, | ||
| "file": { "type": "string", "description": "Relevant for ast_parse, import_statement, call_site." }, | ||
| "line_start": { "type": "integer", "minimum": 1, "description": "1-indexed line number." }, | ||
| "line_end": { "type": "integer", "minimum": 1 }, | ||
| "snippet": { "type": "string", "description": "Short source excerpt (signature line or import statement)." }, | ||
| "pattern": { "type": "string", "description": "Glob/regex pattern (kind=path_pattern)." }, | ||
| "detail": { "type": "string", "description": "Free-text detail (kind=git_diff_stat/name_status)." }, | ||
| "model": { "type": "string", "description": "LLM model id (kind=llm_inference)." }, | ||
| "prompt_ref": { "type": "string", "description": "Internal prompt template reference (kind=llm_inference)." }, | ||
| "temperature": { "type": "number", "minimum": 0, "maximum": 2, "description": "(kind=llm_inference)." }, | ||
| "symbol_ids": { | ||
| "type": "array", | ||
| "items": { "type": "string" }, | ||
| "description": "Symbol IDs that grounded this inferred claim (kind=structural_basis)." | ||
| }, | ||
| "file_ids": { | ||
| "type": "array", | ||
| "items": { "type": "string" }, | ||
| "description": "File IDs that grounded this inferred claim (kind=structural_basis)." | ||
| } | ||
| } | ||
| }, |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Enforce kind-specific evidence payloads.
Evidence accepts kind-only objects such as {"kind": "call_site"} and arbitrary unknown fields. The current producer emits kind-only llm_inference entries, so the promised evidence pointers contain no traceable model, source, or structural basis.
Add additionalProperties: false and conditional required fields for each evidence kind.
🤖 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/schema/diffgraph-v2.schema.json` around lines 101 - 139, Update the
Evidence schema to reject unknown properties with additionalProperties: false,
then add kind-based conditional validation requiring the appropriate payload
fields for each enum value, including model and prompt_ref for llm_inference and
the relevant file, location, pattern, detail, or structural-basis fields for
other kinds. Ensure kind-only evidence objects no longer validate.
Comment on lines
+177
to
+180
| "change_kind": { | ||
| "type": "string", | ||
| "enum": ["added", "modified", "deleted", "renamed", "renamed_modified"] | ||
| }, |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the schema, transformer, and tests mentioning change kinds / unchanged
git ls-files | rg 'diffgraph-v2\.schema\.json|transformer|test|spec' -n
printf '\n--- schema search ---\n'
rg -n '"change_kind"|unchanged|ChangeType\.UNCHANGED|renamed_modified|files\[\]' diffgraph -S
printf '\n--- candidate files around relevant matches ---\n'
rg -n 'ChangeType\.UNCHANGED|unchanged|change_kind' diffgraph -S -A3 -B3Repository: WildestAI/DiffGraph-CLI
Length of output: 12387
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the relevant schema file around the reported lines
file="diffgraph/schema/diffgraph-v2.schema.json"
wc -l "$file"
sed -n '160,210p' "$file" | cat -n
# Show any transformer implementation files that map ChangeType to schema values
rg -n 'ChangeType\.UNCHANGED|change_kind|unchanged' diffgraph -S -A4 -B4Repository: WildestAI/DiffGraph-CLI
Length of output: 14096
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the exact files to inspect if the previous search matched multiple candidates
fd -a 'diffgraph-v2\.schema\.json|transformer|test|spec' diffgraph
# Narrow scan for tests that mention the schema or unchanged file handling
rg -n 'diffgraph-v2\.schema\.json|unchanged|ChangeType\.UNCHANGED|change_kind' diffgraph -S -g '*test*' -g '*spec*' -A2 -B2Repository: WildestAI/DiffGraph-CLI
Length of output: 222
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'FILES\n'
git ls-files | rg 'diffgraph-v2\.schema\.json|transformer|test|spec|diffgraph' -n
printf '\nSCHEMA\n'
file='diffgraph/schema/diffgraph-v2.schema.json'
wc -l "$file"
sed -n '170,190p' "$file" | cat -n
printf '\nMATCHES\n'
rg -n 'ChangeType\.UNCHANGED|unchanged|change_kind' diffgraph -S -A3 -B3Repository: WildestAI/DiffGraph-CLI
Length of output: 11465
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Look for the transformer and any tests around diffgraph serialization
fd -a '.*' diffgraph | rg 'transform|schema|test|spec|diffgraph'Repository: WildestAI/DiffGraph-CLI
Length of output: 739
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect how file_nodes are populated and how file entries are emitted.
sed -n '1,220p' diffgraph/graph_manager.py | cat -n
printf '\n--- structured_export file entry builder ---\n'
sed -n '120,155p' diffgraph/structured_export.py | cat -n
printf '\n--- tests mentioning unchanged or change_kind for file entries ---\n'
rg -n 'unchanged|change_kind|FileEntry|files\[\]' test_cli_manual.sh test_graph_export.py test_structured_export.py tests/test_diffgraph_v2_export.py -S -A3 -B3Repository: WildestAI/DiffGraph-CLI
Length of output: 16025
Add "unchanged" to FileEntry.change_kind. diffgraph/structured_export.py emits "unchanged" for ChangeType.UNCHANGED, and the export tests accept that value, but diffgraph/schema/diffgraph-v2.schema.json:177-180 rejects it. Any unchanged file entry will fail validation unless the enum is updated or those nodes are omitted.
🤖 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/schema/diffgraph-v2.schema.json` around lines 177 - 180, Update the
FileEntry change_kind schema definition to include "unchanged" in its enum,
matching the value emitted by structured_export.py for ChangeType.UNCHANGED.
Preserve all existing enum values.
Comment on lines
+30
to
+68
| def _diff_ref_from_args(diff_args: List[str]) -> Dict[str, Any]: | ||
| """Derive a schema-v2 diff_ref block from raw CLI diff_args.""" | ||
| non_flag = [a for a in diff_args if not a.startswith('-')] | ||
| staged = '--staged' in diff_args or '--cached' in diff_args | ||
|
|
||
| if staged: | ||
| kind = 'staged' | ||
| base_ref = 'HEAD' | ||
| head_ref = None | ||
| elif len(non_flag) >= 2: | ||
| kind = 'commit_range' | ||
| base_ref = non_flag[0] | ||
| head_ref = non_flag[1] | ||
| elif len(non_flag) == 1 and re.match(r'^[^.]+\.{2,3}[^.]+$', non_flag[0]): | ||
| # e.g. main..HEAD or main...HEAD | ||
| parts = re.split(r'\.{2,3}', non_flag[0], maxsplit=1) | ||
| kind = 'commit_range' | ||
| base_ref = parts[0] | ||
| head_ref = parts[1] | ||
| else: | ||
| kind = 'unstaged' | ||
| base_ref = None | ||
| head_ref = None | ||
|
|
||
| # repo_root | ||
| try: | ||
| result = subprocess.run( | ||
| ['git', 'rev-parse', '--show-toplevel'], | ||
| capture_output=True, text=True, check=True | ||
| ) | ||
| repo_root = result.stdout.strip() | ||
| except Exception: | ||
| repo_root = '.' | ||
|
|
||
| # pathspecs (non-flag args that look like paths) | ||
| pathspecs = [ | ||
| a for a in non_flag | ||
| if Path(a).exists() or '/' in a or a.startswith('.') | ||
| ] |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Parse revisions and pathspecs around the -- separator.
The heuristic misclassifies valid arguments: ['main', '--', 'src/app.py'] becomes a commit range from main to the file, refs such as feature/foo become pathspecs, and file_scope is never emitted.
Preserve the separator and parse revision arguments separately from pathspecs.
🧰 Tools
🪛 ast-grep (0.44.1)
[error] 55-58: Command coming from incoming request
Context: subprocess.run(
['git', 'rev-parse', '--show-toplevel'],
capture_output=True, text=True, check=True
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(subprocess-from-request)
🪛 Ruff (0.15.21)
[error] 57-57: Starting a process with a partial executable path
(S607)
[warning] 61-61: 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/structured_export.py` around lines 30 - 68, The _diff_ref_from_args
function must preserve the `--` separator, parse only arguments before it as
revisions, and treat arguments after it exclusively as pathspecs. Update
commit-range detection so refs such as `feature/foo` are not classified as
paths, and emit the parsed pathspecs through the expected `file_scope` field.
Keep staged and unstaged handling intact while ensuring `main -- src/app.py`
represents a single revision plus the file scope, not a commit range.
Comment on lines
+174
to
+184
| # Map component_type to schema v2 SymbolEntry.kind. | ||
| # Allowed: function, method, class, interface, variable, module, other | ||
| kind_map = { | ||
| 'function': 'function', | ||
| 'method': 'method', | ||
| 'class': 'class', | ||
| 'interface': 'interface', | ||
| 'variable': 'variable', | ||
| 'module': 'module', | ||
| } | ||
| kind = kind_map.get(comp.component_type, 'other') |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== Files mentioning SymbolEntry.kind or structured_export ==\n'
git ls-files | rg -n '(^|/)(structured_export\.py|schema|symbol|component|export)' || true
printf '\n== Locate SymbolEntry and component_type ==\n'
rg -n "SymbolEntry|component_type|container|kind_map|type_alias|constant|import" diffgraph . || trueRepository: WildestAI/DiffGraph-CLI
Length of output: 32442
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== structured_export.py around target lines ==\n'
sed -n '150,210p' diffgraph/structured_export.py | cat -n
printf '\n== Search for SymbolEntry schema definitions ==\n'
rg -n "class SymbolEntry|kind:|enum.*kind|symbol.*kind|Allowed: function" diffgraph . || true
printf '\n== Search for component_type documentation ==\n'
rg -n 'component_type == "container"|container|component_type' diffgraph . || trueRepository: WildestAI/DiffGraph-CLI
Length of output: 11642
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== diffgraph-v2.schema.json SymbolEntry section ==\n'
sed -n '207,245p' diffgraph/schema/diffgraph-v2.schema.json | cat -n
printf '\n== tests around symbol kind assertions ==\n'
sed -n '1,140p' tests/test_diffgraph_v2_export.py | cat -nRepository: WildestAI/DiffGraph-CLI
Length of output: 7849
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== graph_manager component_type definition and usage ==\n'
sed -n '1,130p' diffgraph/graph_manager.py | cat -n
printf '\n---\n'
sed -n '210,270p' diffgraph/graph_manager.py | cat -n
printf '\n== structured_export symbol-building callers ==\n'
rg -n "_build_symbol_entries|component_type =|kind_map|SymbolEntry" diffgraph/structured_export.pyRepository: WildestAI/DiffGraph-CLI
Length of output: 11476
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '120,220p' tests/test_diffgraph_v2_export.py | cat -nRepository: WildestAI/DiffGraph-CLI
Length of output: 5048
Map component types to schema-accepted kinds
SymbolEntry.kind only allows function, class, method, import, constant, type_alias, and module. This mapping still emits interface, variable, and the fallback other, so any container/unmapped component will produce invalid output. Align the component_type normalization with the schema and decide what container should map to.
🤖 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/structured_export.py` around lines 174 - 184, Update the kind_map
used by the structured export normalization to emit only schema-accepted
SymbolEntry.kind values: function, class, method, import, constant, type_alias,
or module. Replace the unsupported interface, variable, and other outputs with
appropriate schema kinds, and explicitly decide how container components should
map rather than relying on the invalid fallback.
Comment on lines
+218
to
+225
| entries.append({ | ||
| 'id': rel_id, | ||
| 'kind': 'imports', | ||
| 'source_id': source_id, | ||
| 'target_id': target_id, | ||
| 'analysis_source': 'inferred', | ||
| 'evidence': _LLM_INFERENCE_EVIDENCE, | ||
| }) |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Populate the required confidence for inferred relationships.
The schema requires a numeric confidence whenever analysis_source is "inferred", but both relationship builders omit it. Consequently, any artifact containing an edge fails validation; the schema-validation fixture currently contains no edges and misses this path.
Also applies to: 258-265
🤖 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/structured_export.py` around lines 218 - 225, Update both
relationship builders in structured export to include a numeric confidence value
whenever analysis_source is "inferred". Add the required confidence field to the
entries constructed in the shown imports relationship block and the
corresponding second builder, using the established inference-confidence value
if one exists.
Comment on lines
+359
to
+377
| if validate and _SCHEMA_PATH.exists(): | ||
| try: | ||
| import jsonschema # type: ignore | ||
| with open(_SCHEMA_PATH) as f: | ||
| schema = json.load(f) | ||
| jsonschema.validate(artifact, schema) | ||
| except ImportError: | ||
| pass # jsonschema not installed — skip silently | ||
| except jsonschema.ValidationError as exc: | ||
| import warnings | ||
| warnings.warn( | ||
| f"DiffGraph v2 output failed schema validation: {exc.message}", | ||
| stacklevel=2, | ||
| ) | ||
|
|
||
| out = Path(output_path) | ||
| out.parent.mkdir(parents=True, exist_ok=True) | ||
| with open(out, 'w', encoding='utf-8') as fh: | ||
| json.dump(artifact, fh, indent=2, ensure_ascii=False) |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Do not write an artifact after validation fails.
With validate=True, a ValidationError only emits a warning and execution proceeds to write known-invalid JSON. Re-raise the validation error—or return without writing—so validation actually protects the exported contract.
Proposed fix
except jsonschema.ValidationError as exc:
- import warnings
- warnings.warn(
- f"DiffGraph v2 output failed schema validation: {exc.message}",
- stacklevel=2,
- )
+ raise ValueError(
+ f"DiffGraph v2 output failed schema validation: {exc.message}"
+ ) from exc📝 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.
Suggested change
| if validate and _SCHEMA_PATH.exists(): | |
| try: | |
| import jsonschema # type: ignore | |
| with open(_SCHEMA_PATH) as f: | |
| schema = json.load(f) | |
| jsonschema.validate(artifact, schema) | |
| except ImportError: | |
| pass # jsonschema not installed — skip silently | |
| except jsonschema.ValidationError as exc: | |
| import warnings | |
| warnings.warn( | |
| f"DiffGraph v2 output failed schema validation: {exc.message}", | |
| stacklevel=2, | |
| ) | |
| out = Path(output_path) | |
| out.parent.mkdir(parents=True, exist_ok=True) | |
| with open(out, 'w', encoding='utf-8') as fh: | |
| json.dump(artifact, fh, indent=2, ensure_ascii=False) | |
| if validate and _SCHEMA_PATH.exists(): | |
| try: | |
| import jsonschema # type: ignore | |
| with open(_SCHEMA_PATH) as f: | |
| schema = json.load(f) | |
| jsonschema.validate(artifact, schema) | |
| except ImportError: | |
| pass # jsonschema not installed — skip silently | |
| except jsonschema.ValidationError as exc: | |
| raise ValueError( | |
| f"DiffGraph v2 output failed schema validation: {exc.message}" | |
| ) from exc | |
| out = Path(output_path) | |
| out.parent.mkdir(parents=True, exist_ok=True) | |
| with open(out, 'w', encoding='utf-8') as fh: | |
| json.dump(artifact, fh, indent=2, ensure_ascii=False) |
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 361-361: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(_SCHEMA_PATH)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
[warning] 375-375: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(out, 'w', encoding='utf-8')
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 `@diffgraph/structured_export.py` around lines 359 - 377, The validation branch
in the structured export flow must prevent writing invalid artifacts when
validate is true. Update the jsonschema.ValidationError handler around
jsonschema.validate to re-raise the exception or return before the
output-writing block; preserve successful validation and missing-jsonschema
behavior.
Comment on lines
+327
to
+328
| def test_output_validates_against_schema(self): | ||
| jsonschema = pytest.importorskip('jsonschema') |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -HI '^(pyproject\.toml|setup\.cfg|setup\.py|tox\.ini|requirements.*\.txt)$' . -0 |
xargs -0 -r rg -n -C2 '\b(jsonschema|pytest)\b'
if [ -d .github ]; then
fd -HI '\.ya?ml$' .github -0 |
xargs -0 -r rg -n -C3 '(pip install|requirements|pytest|jsonschema)'
fiRepository: WildestAI/DiffGraph-CLI
Length of output: 165
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate likely dependency and CI files
git ls-files | rg '(^|/)(pyproject\.toml|setup\.cfg|setup\.py|tox\.ini|requirements.*\.txt|Pipfile|poetry\.lock|uv\.lock|\.github/.+\.ya?ml)$'
# Search for jsonschema / importorskip usage in tracked files
git ls-files | xargs rg -n -C2 "\b(jsonschema|pytest\.importorskip\('jsonschema'\)|pytest\.importorskip\(\"jsonschema\"\))" || trueRepository: WildestAI/DiffGraph-CLI
Length of output: 3777
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- tracked candidate files ---'
git ls-files | rg '(^|/)(pyproject\.toml|setup\.cfg|setup\.py|tox\.ini|requirements.*\.txt|Pipfile|poetry\.lock|uv\.lock|\.github/.+\.ya?ml)$' || true
echo '--- jsonschema references ---'
for f in $(git ls-files); do
rg -n "\bjsonschema\b|pytest\.importorskip\(['\"]jsonschema['\"]\)" "$f" && echo "FILE: $f"
done || true
echo '--- pytest.importorskip(jsonschema) locations ---'
rg -n "pytest\.importorskip\(['\"]jsonschema['\"]\)" tests || trueRepository: WildestAI/DiffGraph-CLI
Length of output: 1200
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'PYPROJECTS\n'
fd -HI 'pyproject.toml|setup.cfg|setup.py|tox.ini|requirements.*\\.txt|Pipfile|poetry.lock|uv.lock' .
printf '\nGITHUB WORKFLOWS\n'
fd -HI '\.ya?ml$' .github || true
printf '\nJSONSCHEMA\n'
rg -n --hidden --glob '!**/.git/**' '\bjsonschema\b|pytest\.importorskip\(["'\'']jsonschema["'\'']\)' .Repository: WildestAI/DiffGraph-CLI
Length of output: 1327
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- requirements.txt ---'
cat -n requirements.txt
echo '--- setup.py (dependency-related lines) ---'
rg -n -C3 'install_requires|extras_require|jsonschema|pytest|requirements' setup.pyRepository: WildestAI/DiffGraph-CLI
Length of output: 539
Add jsonschema to the test dependency set
Both schema-validation tests use pytest.importorskip('jsonschema'), so they skip whenever that package is missing. Declare it for the test environment so these contract checks actually run.
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 328-328: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(SCHEMA_PATH)
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_diffgraph_v2_export.py` around lines 327 - 328, Add jsonschema to
the project’s test dependency configuration so the dependency is installed in
the test environment. Ensure both schema-validation tests, including
test_output_validates_against_schema, run instead of being skipped by
pytest.importorskip('jsonschema').
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Release Notes - Version 1.2.0
New Features
--format(html|graph),--graph-format(json|pickle|graphml), and--output, including automatic default paths based on format.Documentation
Tests