Repository navigation
feat: add comment_detail input for the PR comment diagram - #163
Svilen-Stefanov wants to merge 2 commits into
Conversation
The comment's diagram was fixed at the top level, so on a repository with few top-level components most changes lit up the same box. comment_depth draws deeper levels as subgraphs; the default of 1 keeps today's diagram. It changes rendering only: it reaches the event guard and the render step, not depth_cap, the cache keys or the stored analysis. A bad value fails in the guard, before any analysis is paid for. A diagram too large to draw at the requested depth falls back to the top level with a warning rather than failing a review whose analysis already succeeded. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
CodeBoarding reviewStatus: 1 changed component See the full change in CodeBoarding. graph LR
n_action_support_scripts["action_support_scripts"]
classDef added fill:#1f883d,stroke:#0b5d23,color:#ffffff;
classDef modified fill:#bf8700,stroke:#7d4e00,color:#ffffff;
classDef deleted fill:#cf222e,stroke:#82071e,color:#ffffff,stroke-dasharray:5 3;
class n_action_support_scripts modified;
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4b4f9d0150
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| start = ACTION.index(identifier) | ||
| block = ACTION[start : ACTION.index("\n run:", start)] | ||
| self.assertIn("COMMENT_DEPTH: ${{ inputs.comment_depth }}", block) | ||
| readers = {p.name for p in (ROOT / "scripts").rglob("*.*") if "COMMENT_DEPTH" in p.read_text(encoding="utf-8")} |
There was a problem hiding this comment.
In the repository's python -m unittest discover -s tests -v CI job, test discovery imports test_diff_to_mermaid.py and creates scripts/__pycache__/diff_to_mermaid.*.pyc before this test runs. Because rglob("*.*") includes that binary file, read_text(encoding="utf-8") raises UnicodeDecodeError, causing the full suite to fail; restrict the scan to known text/source files or otherwise skip binary and cache files.
Useful? React with 👍 / 👎.
A free depth let the comment draw the whole nested map, which is unreadable at comment width: labels came out at 2.6px on a real analysis. comment_detail offers two views instead. top is today's diagram, byte for byte. specific draws only the deepest changed components, flat, joined by the relations that changed, with an unchanged component only where such a relation ends. A level only holds relations with both ends inside it, so the loader now keeps the unprojected ones: most relations cross parents and exist nowhere else. Ownership of changed code is marked at every level, because a body edit otherwise leaves the nested component that owns it reading as unchanged. It stays out of diff_status, so counts and the top view do not move. specific falls back to top when nothing changed or it overflows Mermaid's caps. That lives in the renderer, so render-review.sh stays one call. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Svilen-Stefanov
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 58dd77dd9e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| for name in (rel.get("src_name"), rel.get("dst_name")): | ||
| if name in known and name not in drawn: | ||
| drawn.add(name) | ||
| components.append(_flat(known[name], "unchanged")) |
There was a problem hiding this comment.
When a changed parent has changed descendants and a changed relation ends at that parent, _deepest_changed initially includes only the descendants, so this branch re-adds the parent but unconditionally marks it unchanged. For example, deleting a parent with children and one of its relations produces a normal, uncolored parent node attached to a red deleted edge; use the endpoint component's existing diff_status rather than forcing unchanged.
Useful? React with 👍 / 👎.
|
|
||
| def index(comps: list) -> None: | ||
| for comp in comps: | ||
| known[_comp_name(comp)] = comp |
There was a problem hiding this comment.
When separate parents contain children with the same name and a changed leaf relation targets one of them, this name-only index overwrites one component with the other. The flattened renderer then resolves the relation by that duplicate name before considering its distinct component_id, so the specific diagram can omit the intended endpoint and attach the edge to the wrong component; index and track flattened endpoints by ID as well as name.
Useful? React with 👍 / 👎.
A trial user on a Java repo (3 top-level components, 7 nested) found that most changes light up the same box in the PR comment. This adds
comment_detail, a small opt-in input with two views. The defaulttopis byte-identical to today, so nobody's comment changes.top(default): every top-level component, as now.specific: only the most specific components that changed, flat, joined by the relations that changed. An unchanged component appears only where a changed relation ends at it.This replaces the numeric
comment_depthfrom the first version of this PR. No free number remains and the action cannot select nested subgraphs, because the full map is unreadable at comment width (table below).Real comparison
render-review.shon the pinned Core 0.15.3, on two consecutive stored analyses fromCodeBoarding/CodeBoarding'scodeboarding/baselinebranch (25ba872e3to38ea16301, 5 components changed). Drawn by Mermaid 11.4.1 in Chromium at 850px, about a GitHub comment's width, with no syntax errors.topspecificLabel size is Mermaid's 16px default times the scale needed to fit. So
specificlabels read at full size,topis small but legible, and depth 2 was not readable without zooming. GitHub's own renderer may differ a little; the diagram at the bottom of this description shows it.All 3 changed relations cross parents, so neither
topnor depth 2 could show them. They roll up onto parent edges that did not change.Decisions worth a look
1.1to2.1exists only as a rolled-up1to2. Here 71 of the 102 relations cross parents.load_analysisnow keeps that list andbuild_diffruns it through the existing_diff_relations.build_diffonly checked it at the top level, so a body edit in a nested component left that component reading asunchanged. On the real pair,Diagram Analysis Pipelinewould have been drawn whole instead ofDiagram Generation OrchestratorandScope Planning and Assembly. It is a separateowns_changefield rather than a status, son_changedandtopdo not move._specific_viewproduces a one-level diff and the existingrender_mermaiddraws it, so the emit code is unchanged. I did not reuse_filter_changed: it keeps ancestors and works per level, which is the opposite of what this view needs.specificfalls back totopif nothing changed (silently) or it overflows Mermaid's caps (with a warning andtruncated=true), so a rendering choice can never fail a review.render-review.shstays one call. On real data repeated 4x with everything changed it falls back; at 2x it fits.depth_cap.inputs.comment_detailreaches the guard and the render step, notdepth_cap, the cache keys, the engine or the stored analysis.test_comment_detail_reaches_rendering_and_nothing_elseholds that in place.review-preview-pr.shis untouched: nothing reads the diagram it writes.Known limits of
specificmodifiedonly because the LLM reworded them ("analyzes CFGs and extracts static analysis results" became "reads static analysis results and call graph structures"). The dependency did not change. The new view makes this more visible, because those edges are now most of the picture.Verification
Tests cover the default,
specific, the fallback, nothing changed, cross-parent relations, nested owners, neighbours and six invalid values. They fail againstmain's code. Locally, unit tests (299, with bytecode on to match CI), provider drift, the Core render step, black, shellcheck and actionlint pass.The first version of this PR failed CI: a test read
.pycfiles as UTF-8, which Codex flagged, and my machine hid it by disabling bytecode. The rewritten test scans source files only.Not tested: a real workflow run of the action end to end, or a real Java analysis (the render uses a Python repo's stored analyses, same format).
comment_detail: specificon the real pair abovegraph LR n_Architecture_Clustering_Service["Architecture Clustering Service"] n_LLM_Scope_Analysis_and_Invocation["LLM Scope Analysis and Invocation"] n_Diagram_Generation_Orchestrator["Diagram Generation Orchestrator"] n_Scope_Planning_and_Assembly["Scope Planning and Assembly"] n_LLM_Scope_Analysis_and_Invocation -- "extracts graph nodes and edges for agent prompt…" --> n_Architecture_Clustering_Service n_Diagram_Generation_Orchestrator -- "reads static analysis results and call graph st…" --> n_Architecture_Clustering_Service n_Scope_Planning_and_Assembly -- "reads static analysis results and call graph st…" --> n_Architecture_Clustering_Service classDef added fill:#1f883d,stroke:#0b5d23,color:#ffffff; classDef modified fill:#bf8700,stroke:#7d4e00,color:#ffffff; classDef deleted fill:#cf222e,stroke:#82071e,color:#ffffff,stroke-dasharray:5 3; class n_Architecture_Clustering_Service,n_LLM_Scope_Analysis_and_Invocation,n_Diagram_Generation_Orchestrator,n_Scope_Planning_and_Assembly modified; linkStyle 0 stroke:#0b5d23,stroke-width:2px; linkStyle 1,2 stroke:#7d4e00,stroke-width:2px;🤖 Generated with Claude Code