DiffGraph Fix: Components within files are actually detected and added to graph. Tooltip to show summary of changes. Colors fixed. #3
Conversation
…eAnalysisAgent - Added a decorator to handle rate limit errors with exponential backoff and jitter. - Refactored agent analysis execution to use the new retry logic. - Enhanced dependency matching for components to allow more flexible name comparisons. - Added functionality to track dependents in component relationships.
- all roadmap documents in the docs folder.
- Updated file and component node styling to use lighter and darker shades respectively for better visual distinction. - Removed redundant color mapping and replaced it with direct class definitions for file and component changes in the Mermaid diagram. - In HTML report, Mermaid diagram is shown before change summary now.
- Added regex-based parsing for component attributes in CodeAnalysisAgent to improve flexibility and robustness. - Updated add_component method in GraphManager to handle summary, dependencies, and dependents more effectively, including cleanup of empty entries. - Improved error handling during component processing to ensure invalid components are skipped without breaking the flow. - Introduced tooltip functionality in HTML report for better user interaction with component nodes.
…am generation and enhance tooltip functionality - Changed Mermaid diagram orientation from TD to LR for better layout. - Improved component ID handling using regex for cleaner formatting. - Enhanced tooltip display in HTML report with markdown parsing and improved styling. - Streamlined event handling for tooltip visibility and interaction with component nodes.
WalkthroughThis update improves the reliability and robustness of the diffgraph-ai system. It introduces exponential backoff retry logic for API calls, refines component parsing and validation, enhances graph construction, and adds interactive tooltips to Mermaid diagrams in HTML reports. Additionally, it provides a detailed roadmap for a future modular multi-agent architecture. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CodeAnalysisAgent
participant OpenAI_API
User->>CodeAnalysisAgent: analyze_changes()
CodeAnalysisAgent->>CodeAnalysisAgent: _run_agent_analysis(prompt) with retry
loop Exponential Backoff on Rate Limit
CodeAnalysisAgent->>OpenAI_API: Call agent
OpenAI_API-->>CodeAnalysisAgent: Response or RateLimitError
end
CodeAnalysisAgent->>CodeAnalysisAgent: Parse agent response (robust parsing)
CodeAnalysisAgent->>CodeAnalysisAgent: Validate and process components
CodeAnalysisAgent-->>User: Analysis result
sequenceDiagram
participant HTMLReport
participant User
participant Browser
HTMLReport->>Browser: Render Mermaid diagram
Browser->>User: Display diagram
User->>Browser: Click on component node
Browser->>Browser: Show tooltip with summary
User->>Browser: Click outside
Browser->>Browser: Hide tooltip
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
diffgraph/graph_manager.py (2)
100-109: Nestedif→ single guard for clarity & SIM102 compliance-if not source or not target or source == target: - return - -if source in self.component_nodes and target in self.component_nodes: - if not self.component_graph.has_edge(source, target): +if ( + not source + or not target + or source == target + or source not in self.component_nodes + or target not in self.component_nodes + or self.component_graph.has_edge(source, target) +): + return + +if not self.component_graph.has_edge(source, target): self.component_graph.add_edge(source, target) @@Streamlines logic and removes Ruff SIM102 warning.
🧰 Tools
🪛 Ruff (0.11.9)
103-104: Use a single
ifstatement instead of nestedifstatements(SIM102)
184-185: Superfluous f-string – triggers Ruff F541-mermaid.append(f' direction TB') +mermaid.append(' direction TB')🧰 Tools
🪛 Ruff (0.11.9)
184-184: f-string without any placeholders
Remove extraneous
fprefix(F541)
docs/Roadmap-v0-demo.md (1)
15-23: Remove trailing punctuation in subheadings
Several “###” subheadings end with a colon, triggering markdownlint MD026. Please drop the trailing:from these headings (e.g., “### 🌟 Objective”, “### 🔹 Definition of Done”, etc.) for consistency and to satisfy the linter.Also applies to: 42-49, 74-78, 106-114, 141-145, 169-171
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
15-15: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
19-19: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
23-23: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
diffgraph/ai_analysis.py(5 hunks)diffgraph/graph_manager.py(3 hunks)diffgraph/html_report.py(3 hunks)docs/Roadmap-v0-demo.md(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
diffgraph/ai_analysis.py (1)
diffgraph/graph_manager.py (3)
ChangeType(7-12)add_component(74-96)add_component_dependency(98-109)
🪛 Ruff (0.11.9)
diffgraph/graph_manager.py
103-104: Use a single if statement instead of nested if statements
(SIM102)
184-184: f-string without any placeholders
Remove extraneous f prefix
(F541)
diffgraph/ai_analysis.py
32-32: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
40-40: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
🪛 Pylint (3.3.7)
diffgraph/graph_manager.py
[error] 200-200: Parsing failed: 'invalid decimal literal (diffgraph.graph_manager, line 200)'
(E0001)
diffgraph/ai_analysis.py
[refactor] 24-24: Either all return statements in a function should return an expression, or none of them should.
(R1710)
🪛 LanguageTool
docs/Roadmap-v0-demo.md
[uncategorized] ~56-~56: Possible missing comma found.
Context: ...atCompletion.createto run each prompt manually first * Define a sharedComponent` sch...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~59-~59: Loose punctuation mark.
Context: ...red Component schema with: * name, type, summary, dependencies, `dep...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~66-~66: Possible missing preposition found.
Context: ...ON with a list of Component objects * Validate that outputs can round-trip through `py...
(AI_HYDRA_LEO_MISSING_TO)
[uncategorized] ~76-~76: Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...bjective: Break large files into chunks so component extraction doesn't fail due t...
(COMMA_COMPOUND_SENTENCE_2)
[uncategorized] ~91-~91: Loose punctuation mark.
Context: ... chunk a metadata block: * chunk_id, start_line, end_line, file_path, ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~157-~157: Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...to reusable tools * Add detailed logging so developers can trace step-by-step agent...
(COMMA_COMPOUND_SENTENCE_2)
[grammar] ~162-~162: Did you mean “to Use”?
Context: ...Memory` between agent steps if needed * Use JSON logs for prompt/output snapshots t...
(MISSING_TO_BEFORE_A_VERB)
🪛 markdownlint-cli2 (0.17.2)
docs/Roadmap-v0-demo.md
15-15: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
19-19: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
23-23: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
29-29: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
42-42: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
49-49: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
53-53: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
61-61: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
74-74: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
78-78: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
86-86: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
94-94: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
106-106: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
114-114: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
122-122: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
128-128: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
141-141: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
145-145: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
153-153: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
159-159: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
171-171: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
175-175: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
179-179: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
185-185: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
🔇 Additional comments (2)
diffgraph/ai_analysis.py (1)
172-188:⚠️ Potential issueColon-splitting bug drops data when a value itself contains “:”
line.split(":")splits all colons, so a summary such as “Handles HTTP 404: Not Found” is truncated. Limit to the first colon.-parts = line.split(":") +parts = line.split(":", 1)Also consider stripping the leading dash (
-) before the split to avoid malformedfield_name.Likely an incorrect or invalid review comment.
docs/Roadmap-v0-demo.md (1)
1-202: Overall: Roadmap document is thorough and well-structured
The stepwise plan for evolving diffgraph-ai into a modular multi-agent system is clear, the goals/statuses are explicit, and the deliverables checklist aligns with the PR objectives.🧰 Tools
🪛 LanguageTool
[uncategorized] ~56-~56: Possible missing comma found.
Context: ...atCompletion.createto run each prompt manually first * Define a sharedComponent` sch...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~59-~59: Loose punctuation mark.
Context: ...redComponentschema with: *name,type,summary,dependencies, `dep...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~66-~66: Possible missing preposition found.
Context: ...ON with a list ofComponentobjects * Validate that outputs can round-trip through `py...(AI_HYDRA_LEO_MISSING_TO)
[uncategorized] ~76-~76: Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...bjective: Break large files into chunks so component extraction doesn't fail due t...(COMMA_COMPOUND_SENTENCE_2)
[uncategorized] ~91-~91: Loose punctuation mark.
Context: ... chunk a metadata block: *chunk_id,start_line,end_line,file_path, ...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~157-~157: Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...to reusable tools * Add detailed logging so developers can trace step-by-step agent...(COMMA_COMPOUND_SENTENCE_2)
[grammar] ~162-~162: Did you mean “to Use”?
Context: ...Memory` between agent steps if needed * Use JSON logs for prompt/output snapshots t...(MISSING_TO_BEFORE_A_VERB)
🪛 markdownlint-cli2 (0.17.2)
15-15: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
19-19: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
23-23: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
29-29: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
42-42: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
49-49: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
53-53: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
61-61: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
74-74: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
78-78: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
86-86: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
94-94: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
106-106: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
114-114: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
122-122: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
128-128: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
141-141: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
145-145: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
153-153: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
159-159: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
171-171: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
175-175: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
179-179: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
185-185: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
| // Tooltip handling | ||
| window.showTooltip = function(text) {{ | ||
| const tooltip = document.getElementById('tooltip'); | ||
| tooltip.innerHTML = marked.parse(text); // Parse markdown in tooltip | ||
| tooltip.classList.add('visible'); | ||
| }} | ||
| window.hideTooltip = function() {{ | ||
| const tooltip = document.getElementById('tooltip'); | ||
| tooltip.classList.remove('visible'); | ||
| }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Tooltip appears but never gets positioned – add coordinates for usable UX
showTooltip() toggles visibility but does not set top/left, so the tooltip is stuck at the page origin. Capture the click location (or target bbox) and position accordingly.
-window.showTooltip = function(text) {
- const tooltip = document.getElementById('tooltip');
- tooltip.innerHTML = marked.parse(text); // Parse markdown in tooltip
- tooltip.classList.add('visible');
+window.showTooltip = function(text, evt) {
+ const tooltip = document.getElementById('tooltip');
+ tooltip.innerHTML = marked.parse(text);
+ tooltip.style.left = `${evt.pageX + 12}px`;
+ tooltip.style.top = `${evt.pageY + 12}px`;
+ tooltip.classList.add('visible');
}(Pass event from the click callback: click id call callback(summary, $event).)
📝 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.
| // Tooltip handling | |
| window.showTooltip = function(text) {{ | |
| const tooltip = document.getElementById('tooltip'); | |
| tooltip.innerHTML = marked.parse(text); // Parse markdown in tooltip | |
| tooltip.classList.add('visible'); | |
| }} | |
| window.hideTooltip = function() {{ | |
| const tooltip = document.getElementById('tooltip'); | |
| tooltip.classList.remove('visible'); | |
| }} | |
| // Tooltip handling | |
| window.showTooltip = function(text, evt) { | |
| const tooltip = document.getElementById('tooltip'); | |
| tooltip.innerHTML = marked.parse(text); | |
| tooltip.style.left = `${evt.pageX + 12}px`; | |
| tooltip.style.top = `${evt.pageY + 12}px`; | |
| tooltip.classList.add('visible'); | |
| } | |
| window.hideTooltip = function() { | |
| const tooltip = document.getElementById('tooltip'); | |
| tooltip.classList.remove('visible'); | |
| } |
🤖 Prompt for AI Agents
In diffgraph/html_report.py around lines 245 to 255, the showTooltip function
makes the tooltip visible but does not set its position, causing it to appear
stuck at the page origin. Modify showTooltip to accept an event parameter, then
use event.clientX and event.clientY or the target element's bounding box to set
the tooltip's CSS top and left properties dynamically. Also, update the click
callback to pass the event object to showTooltip so it can position the tooltip
correctly based on the click location.
There was a problem hiding this comment.
Yeah, but I don't have any way to get the coordinates (or the event object) of the element within the mermaid diagram. If you know how to, please explain (with a link to the documentation supporting your claims).
There was a problem hiding this comment.
Mermaid’s built-in “click” directive only passes the node’s id to the callback:
flowchart LR
A --> B
click A call showTooltip("Some text")
Documentation:
https://mermaid.js.org/syntax/flowchart.html#events-click
“
call functionNamewill call a JavaScript function with the node id as the only argument.”
So the event object is not forwarded.
Here are two practical ways to still get coordinates:
────────────────────────────────────────
- Use the global
window.event(works because the event listener Mermaid creates is synchronous).
window.showTooltip = function (text) {
const e = window.event; // the original click event
const tooltip = document.getElementById('tooltip');
tooltip.innerHTML = marked.parse(text);
tooltip.style.left = `${e.pageX + 12}px`;
tooltip.style.top = `${e.pageY + 12}px`;
tooltip.classList.add('visible');
}• Simple, no extra wiring.
• Supported in all major browsers though considered “old-school”.
────────────────────────────────────────
- Re-bind your own listener after rendering (you’re already calling
bindFunctions).
const { svg, bindFunctions } = await mermaid.render('m1', content);
el.innerHTML = svg;
bindFunctions(el); // keeps Mermaid’s callbacks
// Add custom listener to every node with data-id
el.querySelectorAll('[id]').forEach(node => {
node.addEventListener('click', evt => {
const text = node.getAttribute('title') || '…';
showTooltip(text, evt); // we now have the event!
});
});Now your earlier enhancement works unchanged:
function showTooltip(text, evt) {
const tooltip = document.getElementById('tooltip');
tooltip.innerHTML = marked.parse(text);
tooltip.style.left = `${evt.pageX + 12}px`;
tooltip.style.top = `${evt.pageY + 12}px`;
tooltip.classList.add('visible');
}Mermaid render API docs (returning bindFunctions):
https://mermaid.js.org/config/external.html#browser-usage
This approach avoids relying on window.event and gives full control over positioning.
Pick whichever suits your code style; both require no changes inside Mermaid itself.
- Updated the retry decorator to utilize API rate limit information for more effective delay management. - Implemented fallback mechanisms for delay calculation when 'retry_after' is not available. - Added print statements to log response details during agent analysis execution for better debugging.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
diffgraph/ai_analysis.py (1)
22-55: 🛠️ Refactor suggestionDecorator: still missing
functools.wraps, uses
The same concerns raised in the previous review remain: metadata is lost, console prints are noisy, and the secondexceptblock introduces an unusede. The snippet below fixes all three issues and satisfies Ruff / Pylint.+import functools +import logging +logger = logging.getLogger(__name__) @@ - def wrapper(*args, **kwargs): + @functools.wraps(func) + def wrapper(*args, **kwargs): @@ - print(f"Rate limit hit. Retrying in {delay:.2f} seconds...") + logger.warning("OpenAI rate limit hit. Retry in %.2fs", delay) time.sleep(delay) - except Exception as e: - raise # Re-raise other exceptions immediately + except Exception: + raise # Re-raise non-rate-limit exceptions immediately + # Unreachable, but keeps linters happy + raise openai.RateLimitError("Exceeded maximum retry attempts")🧰 Tools
🪛 Ruff (0.11.9)
51-51: Local variable
eis assigned to but never usedRemove assignment to unused variable
e(F841)
🪛 Pylint (3.3.7)
[refactor] 24-24: Either all return statements in a function should return an expression, or none of them should.
(R1710)
[refactor] 55-55: Too few public methods (1/2)
(R0903)
🧹 Nitpick comments (4)
diffgraph/ai_analysis.py (4)
160-166: Swap debug
Raw agent responses should be emitted vialogger.debug(or removed) to avoid polluting stdout in production environments.- print("--------------------------------") - print(response_text) - print("--------------------------------") + logger.debug("----- Agent response -----\n%s\n--------------------------", response_text)
176-203: Custom parser is brittle; consider YAML parsing for the COMPONENTS block
Rolling your own colon-split logic struggles with edge-cases (multi-line summaries, colons in text, inconsistent spacing). A safer approach is to treat the block as YAML and load it withyaml.safe_load.
Example:import yaml components = yaml.safe_load(components_section) or []This removes the need for all the regex/blank-line bookkeeping and eliminates entire classes of parsing bugs.
224-247: Dependency linking is O(N²); index components for faster look-ups
For large diffs the nested loops will become a hotspot. Build a lowercase name → component_id map once, then resolve dependencies/dependents in O(1) each.name_index = {c.name.lower(): cid for cid, c in self.graph_manager.component_nodes.items()} ... for dep in comp.get("dependencies", []): target_id = name_index.get(dep.lower()) if target_id: self.graph_manager.add_component_dependency(source_id, target_id)
49-51:
Thelogger.warning(see first comment).🧰 Tools
🪛 Ruff (0.11.9)
51-51: Local variable
eis assigned to but never usedRemove assignment to unused variable
e(F841)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
diffgraph/ai_analysis.py(5 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
diffgraph/ai_analysis.py (1)
diffgraph/graph_manager.py (3)
ChangeType(7-12)add_component(74-96)add_component_dependency(98-109)
🪛 Ruff (0.11.9)
diffgraph/ai_analysis.py
51-51: Local variable e is assigned to but never used
Remove assignment to unused variable e
(F841)
🪛 Pylint (3.3.7)
diffgraph/ai_analysis.py
[refactor] 24-24: Either all return statements in a function should return an expression, or none of them should.
(R1710)
- Updated graph_manager to use JSON for component summaries in Mermaid diagrams, enhancing data integrity. - Simplified regex usage for component ID generation in graph edges for cleaner output. - Modified html_report to ensure unique rendering IDs for Mermaid diagrams, preventing conflicts during re-rendering.
Summary by CodeRabbit