docs: S23.13 implementer, facade and consumer relationship diagrams#622
Conversation
Extend the generated post-it relationship diagrams (S23.12) with the reverse and composition views, and fix the link grammar so a name-click always lands on a page carrying its diagram. - reverse view on each implementer page: the role it realises (up) plus the roles it uses (down), with UML multiplicity labels - @optional doc-comment marker (a Doxygen alias) drives the lower bound 0..1 / 0..*; read from the field's trailing comment, so it also renders an "Optional:" badge on the API page - facade class view (SolidSyslog -> the interfaces it uses, with its Config as a linked sidecar) and the config view (Config -> its members) - combined role view: clients above the interface, implementers below - canonical-page grammar: every node links to the compound page where a struct exists, else the file page (opaque handles), and each diagram is injected on that same page, so text-clicks and diagram-clicks agree Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
WalkthroughThe PR expands documentation-time relationship diagrams with reverse implementer, facade, consumer, and combined role views. It adds canonical page-link resolution, optional multiplicity metadata, cached SVG generation, and updated public-header documentation. ChangesRelationship diagram generation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant MkDocs
participant on_page_markdown
participant relationship_data
participant relationship_render
participant APIPage
MkDocs->>on_page_markdown: process API page
on_page_markdown->>relationship_data: build relationship metadata
relationship_data-->>on_page_markdown: return graphs and page targets
on_page_markdown->>relationship_render: render applicable SVG
relationship_render-->>on_page_markdown: return inline SVG
on_page_markdown->>APIPage: prepend diagram
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
☀️ Quality Summary 🚦 build-linux-gcc: 100% successful (✔️ 1522 passed) Created by Quality Monitor v1.14.0 (#f3859fd). More details are shown in the GitHub Checks Result. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
hooks/relationship_data.py (2)
59-92: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winRedundant full header-tree rescans across
build_graph,_defined_structs,build_reverse,build_facade. None of these memoize, and_diagrams()composes them without sharing results, so a single doc build ends up re-globbing and re-regexing every header roughly 5 times over.
hooks/relationship_data.py#L59-L92: add@functools.lru_cache(maxsize=None)tobuild_graph,_defined_structs,build_reverse, andbuild_facade(all pure functions ofroot) so repeated calls within one build are free.hooks/relationship_diagrams.py#L27-L46: no change needed once the builders are memoized —build_consumers/build_graph/build_reverse/build_facadecan keep being called independently here.🤖 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 `@hooks/relationship_data.py` around lines 59 - 92, Memoize the pure root-based builders build_graph, _defined_structs, build_reverse, and build_facade with functools.lru_cache(maxsize=None) so repeated calls reuse results within a documentation build. In hooks/relationship_data.py lines 59-92, add the decorators and required functools import; hooks/relationship_diagrams.py lines 27-46 requires no direct change because its independent builder calls will then benefit from caching.
43-198: 📐 Maintainability & Code Quality | 🔵 TrivialBundle of low-value Ruff hints (non-blocking).
Several private helpers lack return-type annotations (
_defined_structsL43,_canonicalL52,_balancedL100,_regionL119,_collaboratorsL128 — ANN202), and threeopen(...)calls pass the redundant default"r"mode (L47, L135, L198 — UP015). Worth aruff --fixpass at some point; not urgent for a doc-build script.🤖 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 `@hooks/relationship_data.py` around lines 43 - 198, The private helpers _defined_structs, _canonical, _balanced, _region, and _collaborators need explicit return-type annotations; add accurate annotations without changing behavior. Remove the redundant "r" mode arguments from the open calls in _defined_structs, _collaborators, and build_facade, relying on open’s default text-read mode.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 `@hooks/relationship_render.py`:
- Around line 102-297: Apply Ruff’s safe automatic fixes across the affected
helpers, including converting eligible `.format()` calls to f-strings and adding
`strict=` to the `zip()` calls in `_place_collaborators` and related code.
Address boolean positional-argument warnings with explicit keywords where
supported, and replace intentionally unused unpacked values such as `fac_page`
and `cfg_page` with `_` while preserving the current rendering behavior.
- Around line 353-378: The width calculation around cons_cx and all_cx does not
account for the leftmost consumer boundary, so wide consumer rows can be
clipped. Recompute the horizontal layout origin using both implementer and
consumer extents, or shift all related x-coordinates right, then derive width
from the resulting leftmost and rightmost bounds so every consumer and
implementer remains inside the viewBox.
---
Outside diff comments:
In `@hooks/relationship_data.py`:
- Around line 59-92: Memoize the pure root-based builders build_graph,
_defined_structs, build_reverse, and build_facade with
functools.lru_cache(maxsize=None) so repeated calls reuse results within a
documentation build. In hooks/relationship_data.py lines 59-92, add the
decorators and required functools import; hooks/relationship_diagrams.py lines
27-46 requires no direct change because its independent builder calls will then
benefit from caching.
- Around line 43-198: The private helpers _defined_structs, _canonical,
_balanced, _region, and _collaborators need explicit return-type annotations;
add accurate annotations without changing behavior. Remove the redundant "r"
mode arguments from the open calls in _defined_structs, _collaborators, and
build_facade, relying on open’s default text-read mode.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d89ed99d-354d-407b-865b-4d84de1dcc03
📒 Files selected for processing (6)
Core/Interface/SolidSyslogBlockStore.hCore/Interface/SolidSyslogConfig.hhooks/relationship_data.pyhooks/relationship_diagrams.pyhooks/relationship_render.pymkdocs.yml
- fix consumer-row clipping in the combined role view: shift all x so the leftmost box (consumer or interface) sits at the margin, and derive width from both the left and right extents rather than the rightmost box alone - convert .format() calls to f-strings across the render module (ruff UP032) - memoize the pure header-scanning builders with lru_cache so a build's repeated calls collapse to one header rescan - drop redundant open() "r" modes, prefix the unused unpacked page vars, add zip(strict=True) to the equal-length pairings Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Thanks @coderabbitai — addressed the review in Functional (major): consumer-row clipping in the combined role view — fixed by shifting all x-coordinates so the leftmost box sits at the margin and computing width from both extents. Verified with a synthetic wide-consumer case; real roles are byte-identical. Maintainability (render module): applied f-strings (UP032), redundant Performance (data module): memoized Consciously deferred (nitpicks): ANN202 return-type annotations and FBT003 boolean-positional on internal helpers of a build-time script — non-blocking, and I've kept them out to avoid noise.
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
git log --oneline -5
git show --stat a57fb0e 2>&1 | head -50Length of output: 1581 🏁 Script executed: #!/bin/bash
git show a57fb0e -- hooks/relationship_render.py | head -200Length of output: 14087 🏁 Script executed: #!/bin/bash
git show a57fb0e -- hooks/relationship_data.py
echo "-----render clipping-----"
git show a57fb0e -- hooks/relationship_render.py | grep -n "leftmost\|margin\|width\|render_role_combined" | head -60Length of output: 10843 🏁 Script executed: #!/bin/bash
sed -n '330,410p' hooks/relationship_render.pyLength of output: 3960 The commit checks out.
One thing worth double-checking on your side since it's not visible from the diff: No further action needed on my side — this addresses all four points raised. (=^･ω･^=) tidy work, David |
☀️ Quality Summary 🚦 build-linux-gcc: 100% successful (✔️ 1522 passed) Created by Quality Monitor v1.14.0 (#f3859fd). More details are shown in the GitHub Checks Result. |
1 participant
Follow-on to S23.12 (#619): the reverse of the "realised by" diagrams, plus the facade/config composition views and a link-grammar fix — all generated from the headers at docs-build time.
What's new
0..1/1..*/0..*).@optionalmarker — a Doxygen alias in the config doc-comments drives the multiplicity lower bound and renders an "Optional:" badge on the API page; read from the field's trailing comment.Verification
mkdocs build --strictgreen.Address/facade stay on file pages.Closes #621
🤖 Generated with Claude Code
Summary by CodeRabbit
@optionalconsistently for diagram build-time inference.