Print Mermaid diagram schema names in CLI output - #25
Conversation
Reviewer's GuideRefactors diagram parsing to produce structured Diagram objects capturing source, line numbers, and schema; updates the CLI to emit per-diagram start/end markers with schema and line bounds; adds an integration test for schema reporting; adjusts unit tests; and documents the new markers in the README. Sequence diagram for CLI per-diagram processing and reportingsequenceDiagram
participant CLI
participant File
participant Diagram
participant Renderer
actor User
User->>CLI: Run CLI on Markdown file
CLI->>File: Read file contents
CLI->>Diagram: parse_blocks(text)
loop For each Diagram
CLI->>CLI: Print --> line {line_start}: {schema}
CLI->>Renderer: render_block(source, ...)
Renderer-->>CLI: Render result
CLI->>CLI: Print <-- line {line_end}: {schema}
end
Class diagram for the new Diagram data structureclassDiagram
class Diagram {
+str source
+int line_start
+int line_end
+str schema
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Note Reviews pausedUse the following commands to manage reviews:
Summary by CodeRabbit
WalkthroughSummarise the change: switch CLI rendering from concurrent to sequential per‑diagram; add a frozen, slots-enabled Diagram dataclass with positional and schema metadata; emit per‑diagram start/end markers (line numbers + schema) around rendering; update parsing, tests, and README to reflect marker semantics and removed concurrency option. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant CLI as nixie.check_file
participant P as parse_blocks
participant R as render_block
U->>CLI: invoke CLI on file(s)
CLI->>P: parse_blocks(file_text)
P-->>CLI: [Diagram{source, line_start, line_end, schema}]*
alt no diagrams
CLI-->>U: exit success
else diagrams found
loop For each Diagram (sequential)
CLI->>CLI: print "--> line X: schema" (stdout, flush)
CLI->>R: render_block(diagram.source)
note right of R #F7F7D9: render output/errors → stderr\nmay interleave with stdout markers
R-->>CLI: render result / exception
CLI->>CLI: print "<-- line Y: schema" (stdout, flush)
end
CLI-->>U: exit with aggregated status
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
✨ Finishing Touches🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
nixie/unittests/test_parse_blocks.py (1)
1-39: Add a unit test for skipping Mermaid config/comment linesAdd a focused unit test that ensures schema detection skips '%%' config lines and obtains the actual diagram schema.
Proposed new test (place after existing tests):
def test_parse_blocks_skips_config_lines() -> None: content = """```mermaid %%{init: {'theme': 'forest'}}%% sequenceDiagram A->>B: hello ```""" diagrams = parse_blocks(content) assert [d.schema for d in diagrams] == ["sequenceDiagram"] assert [d.source for d in diagrams] == ["%%{init: {'theme': 'forest'}}%%\nsequenceDiagram\nA->>B: hello"] assert [d.line_start for d in diagrams] == [2] assert [d.line_end for d in diagrams] == [5]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- Jira integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
README.md(1 hunks)nixie/cli.py(3 hunks)nixie/unittests/test_parse_blocks.py(1 hunks)tests/integration/test_cli_behavior.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.py
📄 CodeRabbit Inference Engine (AGENTS.md)
**/*.py: Python changes must pass tests (unit and behavioral) before completion/commit
Python code must pass lint checks (make lint)
Python code must adhere to formatting standards (make check-fmt/make fmt)
Python code must pass type checking (make typecheck)
Follow core Python 3.13 style conventions per .rules/python-00.md
Apply best practices for context managers per .rules/python-context-managers.md
Follow generator and iterator patterns per .rules/python-generators.md
Follow function return conventions per .rules/python-return.md
Apply Python typing best practices per .rules/python-typing.md
**/*.py: Name Python files in snake_case (e.g., http_client.py, task_queue.py)
Classes must use PascalCase
Variables and functions must use snake_case
Module-level constants use UPPER_SNAKE_CASE
Prefix non-exported helpers or internal APIs with a single underscore
Use typing everywhere; maintain full static type coverage with Pyright
Use TypedDict or @DataClass for structured data; prefer @DataClass(slots=True) for internal-only
Avoid Any; use Unknown, generics, or cast() with justification if Any is used
Be explicit with return types for all public functions and class methods (e.g., -> None, -> str)
Favor immutability (prefer tuples to lists; use frozendict or types.MappingProxyType where appropriate)
Use# pyright: ignoresparingly and include an explanation when used
Avoid side effects at import time; modules should not modify global state or perform actions on import
Never hardcode secrets in source code
Write NumPy-style docstrings for public functions, classes, and modules
Add inline comments to explain non-obvious logic or decisions
**/*.py: Use context managers to encapsulate setup/teardown for resources (files, locks, connections) instead of manual management
Use @contextmanager from contextlib for straightforward, linear setup/teardown without persistent internal state
Implement a class-based context manager (enter/exit) when there is internal sta...
Files:
tests/integration/test_cli_behavior.pynixie/cli.pynixie/unittests/test_parse_blocks.py
⚙️ CodeRabbit Configuration File
**/*.py: - Keep cyclomatic complexity ≤ 12
- Follow single responsibility and CQRS (command/query segregation)
- Docstrings must follow the
numpystyle guide. Use a single-line summary for private functions and methods, and full structured docs for all public interfaces.- Move conditionals with >2 branches to predicate/helper functions
- Avoid
eval,exec,pickle, monkey-patching,ctypes, unsafe shell- Every module must begin with a triple-quoted docstring explaining its purpose, utility, and usage, including example calls if appropriate.
- Comments and docs must follow en-GB-oxendict (-ize / -our) spelling and grammar
- Lint suppressions:
- Blanket
# noqa, file-level skips, and categories are forbidden- Only narrow in-line disables (
# noqa: XYZ) are permitted, and must be accompanied byFIXME:or a ticket link, and used only as a last resort.- Use
pytestfixtures for shared setup (conftest.pyorfixtures/)- Replace duplicate tests with
@pytest.mark.parametrize- Prefer
pytest-mockorunittest.mockfor stubs/mocks- Use
assert …, "message"over bare asserts- Reflect all API/behaviour changes in
docs/and update roadmap on completion- Files must not exceed 400 logical lines:
- Decompose large modules into subpackages
- Split large
match/caseor dispatch tables by domain and collocate with targets if appropriate- Move bulky data (fixtures, templates) to external files for parsing at runtime
- Mutable defaults and shadowed built-ins are forbidden
- All code must have clear type hints using modern style (
A | B,list[str],class Foo[A]:,type Bar = int, etc.), with ABC imports drawn from the correct stdlib module.
Files:
tests/integration/test_cli_behavior.pynixie/cli.pynixie/unittests/test_parse_blocks.py
tests/integration/test_*.py
📄 CodeRabbit Inference Engine (.rules/python-00.md)
Place integration tests under tests/integration/ with files prefixed with test_
Files:
tests/integration/test_cli_behavior.py
{**/unittests/test_*.py,tests/integration/test_*.py}
📄 CodeRabbit Inference Engine (.rules/python-00.md)
{**/unittests/test_*.py,tests/integration/test_*.py}: Use pytest idioms: prefer fixtures, parametrize broadly, avoid unnecessary mocks
Group related tests using classes with method names prefixed by test_
Write tests from a user's perspective; test public behaviour, not internals
Avoid excessive mocking; use doubles only for external services or non-deterministic behaviour
Files:
tests/integration/test_cli_behavior.pynixie/unittests/test_parse_blocks.py
**/*.md
📄 CodeRabbit Inference Engine (AGENTS.md)
**/*.md: Markdown files must pass markdown linting (make markdownlint)
Markdown files containing Mermaid diagrams must pass nixie validation (make nixie)
Files:
README.md
⚙️ CodeRabbit Configuration File
**/*.md: * Avoid 2nd person or 1st person pronouns ("I", "you", "we")
- Use en-GB-oxendict (-ize / -our) spelling and grammar
- Headings must not be wrapped.
- Documents must start with a level 1 heading
- Headings must correctly increase or decrease by no more than one level at a time
- Use GitHub-flavoured Markdown style for footnotes and endnotes.
- Numbered footnotes must be numbered by order of appearance in the document.
Files:
README.md
**/README.md
📄 CodeRabbit Inference Engine (.rules/python-00.md)
Colocate README.md near reusable packages and include usage examples
Files:
README.md
**/unittests/test_*.py
📄 CodeRabbit Inference Engine (.rules/python-00.md)
Colocate unit tests with code in an unittests/ subdirectory using files prefixed with test_
Files:
nixie/unittests/test_parse_blocks.py
🧬 Code Graph Analysis (2)
tests/integration/test_cli_behavior.py (2)
tests/integration/conftest.py (1)
stub_render(12-31)nixie/cli.py (1)
main(439-455)
nixie/unittests/test_parse_blocks.py (1)
nixie/cli.py (1)
parse_blocks(80-91)
🔇 Additional comments (3)
nixie/cli.py (1)
20-20: LGTM: adopt dataclasses under a short aliasThe alias improves readability in this module and is applied consistently. No issues.
README.md (2)
62-64: Clarify that diagram markers include schema and line numbersThe documentation correctly communicates the new markers. Keep the phrasing—it matches the CLI output and tests.
67-69: Show both start and end markers in the exampleThe example reads clearly and matches the CLI behaviour and tests.
|
@coderabbitai Have the following now been resolved?
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain@leynos: Looking at the current implementation, neither of the issues have been fully resolved: 1. Concurrent Output Interleaving Issue - Not ResolvedThe current code still uses direct async def process(diagram: Diagram) -> bool:
print(f"--> line {diagram.line_start}: {diagram.schema}")
success = await render_block(diagram.source)
print(f"<-- line {diagram.line_end}: {diagram.schema}")
return successThese 2. Schema Detection Logic - Not ResolvedThe schema detection still uses simple whitespace splitting: schema = first_line.split()[0] if first_line.split() else "unknown"This approach has several issues:
RecommendationsFor concurrent output:
For schema detection:
🏁 Script executed: Length of output: 17812 🏁 Script executed: Length of output: 662 🏁 Script executed: Length of output: 3190 @leynos: After examining the current implementation, I have good news - both issues have actually been resolved: 1. Concurrent Output Interleaving Issue - ✅ ResolvedThe code no longer uses concurrent tasks for diagram processing. Instead, it processes diagrams sequentially within each file: async def check_file(path: Path, cfg_path: Path | None, semaphore: asyncio.Semaphore) -> bool:
diagrams = parse_blocks(path.read_text(encoding="utf-8"))
if not diagrams:
return True
with tempfile.TemporaryDirectory() as tmpdir:
tmp_path = Path(tmpdir)
all_success = True
for idx, diagram in enumerate(diagrams, 1): # Sequential processing
with diagram_markers(diagram): # Thread-safe context manager
try:
success = await render_block(...) # Only the CLI call is async
except Exception:
success = False
if not success:
all_success = False
return all_successThe @contextmanager
def diagram_markers(diagram: Diagram) -> typ.Generator[None, None, None]:
"""Print markers bracketing ``diagram`` processing."""
print(f"--> line {diagram.line_start}: {diagram.schema}")
try:
yield
finally:
print(f"<-- line {diagram.line_end}: {diagram.schema}")2. Schema Detection Logic - ✅ ResolvedThe schema detection has been significantly improved with a dedicated def _extract_schema(lines: list[str]) -> str:
"""Return the schema name from ``lines``.
Mermaid diagrams may start with empty lines or comments beginning with
``%%``. Skip these until a meaningful line is found. If no schema can be
determined, return ``UNKNOWN_SCHEMA``.
"""
for line in lines:
stripped = line.strip()
if not stripped or stripped.startswith("%%"): # Skip empty lines and comments
continue
return stripped.split()[0]
return UNKNOWN_SCHEMA # Meaningful defaultThis robust implementation:
Both concerns have been thoroughly addressed in the current implementation! |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
nixie/unittests/test_parse_blocks.py (1)
44-61: Assert line numbers in the empty/whitespace/comment casesComplete the contract by checking
line_startandline_endin these cases.diag_empty = parse_blocks(content_empty) assert len(diag_empty) == 1 assert diag_empty[0].schema == UNKNOWN_SCHEMA assert diag_empty[0].source == "" +assert diag_empty[0].line_start == 2 +assert diag_empty[0].line_end == 3 diag_ws = parse_blocks(content_ws) assert len(diag_ws) == 1 assert diag_ws[0].schema == UNKNOWN_SCHEMA assert diag_ws[0].source == " " +assert diag_ws[0].line_start == 2 +assert diag_ws[0].line_end == 3 content_comment = "```mermaid\n%% a comment\nsequenceDiagram\nA->B\n```" diag_comment = parse_blocks(content_comment) assert diag_comment[0].schema == "sequenceDiagram" +assert diag_comment[0].line_start == 2 +assert diag_comment[0].line_end == 5
♻️ Duplicate comments (2)
README.md (1)
72-76: State that markers are flushed immediatelySet expectations for log consumers by noting that markers are flushed, which helps bracketing stderr reliably in terminals/CI.
- lines for that diagram. Markers are printed on stdout; messages from + lines for that diagram. Markers are printed on stdout and are flushed + immediately; messages fromnixie/unittests/test_parse_blocks.py (1)
19-22: Verify line numbers for the single‑block variationsLock in the documented contract by asserting
line_start/line_endfor these inputs as well.diagrams = parse_blocks(text) assert [d.source for d in diagrams] == ["A-->B"] assert [d.schema for d in diagrams] == ["A-->B"] +assert [d.line_start for d in diagrams] == [2] +assert [d.line_end for d in diagrams] == [3]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- Jira integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
README.md(1 hunks)nixie/cli.py(3 hunks)nixie/unittests/test_diagram_markers.py(1 hunks)nixie/unittests/test_parse_blocks.py(3 hunks)tests/integration/test_cli_behavior.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
⚙️ CodeRabbit configuration file
**/*.py: - Keep cyclomatic complexity ≤ 12
- Follow single responsibility and CQRS (command/query segregation)
- Docstrings must follow the
numpystyle guide. Use a single-line summary for private functions and methods, and full structured docs for all public interfaces.- Move conditionals with >2 branches to predicate/helper functions
- Avoid
eval,exec,pickle, monkey-patching,ctypes, unsafe shell- Every module must begin with a triple-quoted docstring explaining its purpose, utility, and usage, including example calls if appropriate.
- Comments and docs must follow en-GB-oxendict (-ize / -our) spelling and grammar
- Lint suppressions:
- Blanket
# noqa, file-level skips, and categories are forbidden- Only narrow in-line disables (
# noqa: XYZ) are permitted, and must be accompanied byFIXME:or a ticket link, and used only as a last resort.- Use
pytestfixtures for shared setup (conftest.pyorfixtures/)- Replace duplicate tests with
@pytest.mark.parametrize- Prefer
pytest-mockorunittest.mockfor stubs/mocks- Use
assert …, "message"over bare asserts- Reflect all API/behaviour changes in
docs/and update roadmap on completion- Files must not exceed 400 logical lines:
- Decompose large modules into subpackages
- Split large
match/caseor dispatch tables by domain and collocate with targets if appropriate- Move bulky data (fixtures, templates) to external files for parsing at runtime
- Mutable defaults and shadowed built-ins are forbidden
- All code must have clear type hints using modern style (
A | B,list[str],class Foo[A]:,type Bar = int, etc.), with ABC imports drawn from the correct stdlib module.
Files:
nixie/cli.pynixie/unittests/test_diagram_markers.pynixie/unittests/test_parse_blocks.pytests/integration/test_cli_behavior.py
**/*.md
⚙️ CodeRabbit configuration file
**/*.md: * Avoid 2nd person or 1st person pronouns ("I", "you", "we")
- Use en-GB-oxendict (-ize / -our) spelling and grammar
- Headings must not be wrapped.
- Documents must start with a level 1 heading
- Headings must correctly increase or decrease by no more than one level at a time
- Use GitHub-flavoured Markdown style for footnotes and endnotes.
- Numbered footnotes must be numbered by order of appearance in the document.
Files:
README.md
🔇 Additional comments (3)
nixie/unittests/test_parse_blocks.py (1)
27-31: LGTM: multi‑block extraction including line numbersnixie/cli.py (2)
70-85: LGTM: immutable Diagram with documented fieldsFreezing and documenting the dataclass aligns with the immutability and clarity goals.
124-132: LGTM: marker context manager with immediate flushThe context manager brackets work neatly and solves interleaving around stderr output in practice.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
nixie/cli.py (2)
353-355: Write Mermaid source with explicit encodingAvoid platform-default encodings when writing the
.mmdfile.Apply this diff:
- mmd.write_text(block) + mmd.write_text(block, encoding="utf-8")
436-438: Fix invalid isinstance use of PEP 604 unions
isinstance(exc, KeyboardInterrupt | SystemExit)raisesTypeError. Use a tuple of types.Apply this diff:
- if isinstance(exc, KeyboardInterrupt | SystemExit): + if isinstance(exc, (KeyboardInterrupt, SystemExit)): raise
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- Jira integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
README.md(3 hunks)nixie/cli.py(3 hunks)nixie/unittests/test_diagram_markers.py(1 hunks)nixie/unittests/test_parse_blocks.py(3 hunks)tests/integration/test_cli_behavior.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Python changes must pass tests (unit and behavioral) before completion/commit
Python code must pass lint checks (make lint)
Python code must adhere to formatting standards (make check-fmt/make fmt)
Python code must pass type checking (make typecheck)
Follow core Python 3.13 style conventions per .rules/python-00.md
Apply best practices for context managers per .rules/python-context-managers.md
Follow generator and iterator patterns per .rules/python-generators.md
Follow function return conventions per .rules/python-return.md
Apply Python typing best practices per .rules/python-typing.md
**/*.py: Name Python files in snake_case (e.g., http_client.py, task_queue.py)
Classes must use PascalCase
Variables and functions must use snake_case
Module-level constants use UPPER_SNAKE_CASE
Prefix non-exported helpers or internal APIs with a single underscore
Use typing everywhere; maintain full static type coverage with Pyright
Use TypedDict or @DataClass for structured data; prefer @DataClass(slots=True) for internal-only
Avoid Any; use Unknown, generics, or cast() with justification if Any is used
Be explicit with return types for all public functions and class methods (e.g., -> None, -> str)
Favor immutability (prefer tuples to lists; use frozendict or types.MappingProxyType where appropriate)
Use# pyright: ignoresparingly and include an explanation when used
Avoid side effects at import time; modules should not modify global state or perform actions on import
Never hardcode secrets in source code
Write NumPy-style docstrings for public functions, classes, and modules
Add inline comments to explain non-obvious logic or decisions
**/*.py: Use context managers to encapsulate setup/teardown for resources (files, locks, connections) instead of manual management
Use @contextmanager from contextlib for straightforward, linear setup/teardown without persistent internal state
Implement a class-based context manager (enter/exit) when there is internal sta...
Files:
nixie/unittests/test_diagram_markers.pynixie/cli.pytests/integration/test_cli_behavior.pynixie/unittests/test_parse_blocks.py
⚙️ CodeRabbit configuration file
**/*.py: - Keep cyclomatic complexity ≤ 12
- Follow single responsibility and CQRS (command/query segregation)
- Docstrings must follow the
numpystyle guide. Use a single-line summary for private functions and methods, and full structured docs for all public interfaces.- Move conditionals with >2 branches to predicate/helper functions
- Avoid
eval,exec,pickle, monkey-patching,ctypes, unsafe shell- Every module must begin with a triple-quoted docstring explaining its purpose, utility, and usage, including example calls if appropriate.
- Comments and docs must follow en-GB-oxendict (-ize / -our) spelling and grammar
- Lint suppressions:
- Blanket
# noqa, file-level skips, and categories are forbidden- Only narrow in-line disables (
# noqa: XYZ) are permitted, and must be accompanied byFIXME:or a ticket link, and used only as a last resort.- Use
pytestfixtures for shared setup (conftest.pyorfixtures/)- Replace duplicate tests with
@pytest.mark.parametrize- Prefer
pytest-mockorunittest.mockfor stubs/mocks- Use
assert …, "message"over bare asserts- Reflect all API/behaviour changes in
docs/and update roadmap on completion- Files must not exceed 400 logical lines:
- Decompose large modules into subpackages
- Split large
match/caseor dispatch tables by domain and collocate with targets if appropriate- Move bulky data (fixtures, templates) to external files for parsing at runtime
- Mutable defaults and shadowed built-ins are forbidden
- All code must have clear type hints using modern style (
A | B,list[str],class Foo[A]:,type Bar = int, etc.), with ABC imports drawn from the correct stdlib module.
Files:
nixie/unittests/test_diagram_markers.pynixie/cli.pytests/integration/test_cli_behavior.pynixie/unittests/test_parse_blocks.py
**/unittests/test_*.py
📄 CodeRabbit inference engine (.rules/python-00.md)
Colocate unit tests with code in an unittests/ subdirectory using files prefixed with test_
Files:
nixie/unittests/test_diagram_markers.pynixie/unittests/test_parse_blocks.py
{**/unittests/test_*.py,tests/integration/test_*.py}
📄 CodeRabbit inference engine (.rules/python-00.md)
{**/unittests/test_*.py,tests/integration/test_*.py}: Use pytest idioms: prefer fixtures, parametrize broadly, avoid unnecessary mocks
Group related tests using classes with method names prefixed by test_
Write tests from a user's perspective; test public behaviour, not internals
Avoid excessive mocking; use doubles only for external services or non-deterministic behaviour
Files:
nixie/unittests/test_diagram_markers.pytests/integration/test_cli_behavior.pynixie/unittests/test_parse_blocks.py
**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
**/*.md: Markdown files must pass markdown linting (make markdownlint)
Markdown files containing Mermaid diagrams must pass nixie validation (make nixie)
Files:
README.md
⚙️ CodeRabbit configuration file
**/*.md: * Avoid 2nd person or 1st person pronouns ("I", "you", "we")
- Use en-GB-oxendict (-ize / -our) spelling and grammar
- Headings must not be wrapped.
- Documents must start with a level 1 heading
- Headings must correctly increase or decrease by no more than one level at a time
- Use GitHub-flavoured Markdown style for footnotes and endnotes.
- Numbered footnotes must be numbered by order of appearance in the document.
Files:
README.md
**/README.md
📄 CodeRabbit inference engine (.rules/python-00.md)
Colocate README.md near reusable packages and include usage examples
Files:
README.md
tests/integration/test_*.py
📄 CodeRabbit inference engine (.rules/python-00.md)
Place integration tests under tests/integration/ with files prefixed with test_
Files:
tests/integration/test_cli_behavior.py
🧬 Code graph analysis (3)
nixie/unittests/test_diagram_markers.py (1)
nixie/cli.py (3)
cli(541-558)Diagram(72-91)diagram_markers(131-137)
tests/integration/test_cli_behavior.py (2)
tests/integration/conftest.py (1)
stub_render(12-31)nixie/cli.py (1)
main(485-501)
nixie/unittests/test_parse_blocks.py (1)
nixie/cli.py (2)
cli(541-558)parse_blocks(112-127)
🔍 Remote MCP
Here is the additional concrete context from the PR diff that will be directly relevant to code review:
• README.md updates
– The old --concurrency flag is now documented as “reserved for future use,” and diagrams are processed strictly sequentially within each file to guarantee bracketed start/end markers. The per‐diagram markers now include both line numbers and the schema name, with errors emitted between the markers on stderr while the markers themselves go to stdout (github.com)
• nixie/cli.py changes
– New imports: bisect and dataclasses as dc.
– Added
• @dc.dataclass Diagram (slots-enabled, frozen) with fields
– source: str – raw Mermaid text (no backticks)
– line_start: int – 1-based start line of the block content
– line_end: int – 1-based line of the closing fence
– schema: str – first meaningful token or fallback UNKNOWN_SCHEMA
• UNKNOWN_SCHEMA: typ.Final[str] = "" constant
• _extract_schema(lines: list[str]) -> str helper: skips blank lines and those starting with %%, returns first token or UNKNOWN_SCHEMA if none found
• parse_blocks(text: str) -> list[Diagram]:
– Precomputes newline_offsets via re.finditer("\n", text) to avoid O(N²) behavior (github.com)
– Uses bisect.bisect_left on newline_offsets to map block start to line_start
– Calculates line_end = line_start + max(len(lines), 1)
– Extracts schema with _extract_schema and returns a list of Diagram instances (github.com)
• @contextmanager diagram_markers(diagram: Diagram): prints
– --> line {line_start}: {schema} before processing
– <-- line {line_end}: {schema} on exit (even on exception), with flush=True (github.com)
• check_file(...) now loops over diagrams sequentially (rather than asyncio.gather), wrapping each call to render_block in diagram_markers and collecting an overall success flag (github.com)
• Test updates
– test_parse_blocks.py:
• Now asserts on Diagram.source, .schema, .line_start, .line_end and use of UNKNOWN_SCHEMA for empty/whitespace-only blocks (github.com)
– Added test_diagram_markers.py:
• Verifies both markers are printed (and flushed) and that the end marker is printed even if an exception occurs within the context (github.com)
– Added integration test test_cli_reports_diagram_schemas:
• Writes a Markdown file with two diagrams, runs the CLI at concurrency=1 to assert the four markers (start/end for each diagram) appear exactly once and in ascending order; repeats at concurrency=2 to ensure markers still appear exactly once (order may vary) (github.com)
These points highlight the new data model (Diagram), schema‐extraction logic, output‐bracketing mechanism, shift to sequential processing for stable output, and the corresponding unit and integration tests that enforce the new behavior.
🔇 Additional comments (15)
nixie/cli.py (5)
20-21: Imports for bisect/dataclasses are appropriateKeep. These are required by
parse_blocksandDiagram.
71-92: Solid immutable Diagram model with clear docsThe frozen, slots-enabled dataclass with explicit attributes and a docstring aligns with the repo’s typing/immutability rules.
94-109: Schema extraction correctly skips blanks and Mermaid commentsThe helper handles
%%comment/config lines and falls back toUNKNOWN_SCHEMA. Keep this behaviour; it prevents misleading schemas from comment lines.
112-128: Avoid O(N²) line-numbering; off‑by‑one semantics look correctPrecomputing newline offsets and using
bisect_leftfixes the previous scaling issue.line_start = (#newlines before block) + 1andline_end = line_start + max(len(lines), 1)match the documented contract.
130-138: Bracket markers with flush=True is correctFlushing ensures markers surround any stderr reliably and fixes the earlier interleaving concern. Good use of a context manager for DRYness.
nixie/unittests/test_diagram_markers.py (2)
12-22: Assert start/end marker prints with flushingThe test validates both markers and
flush=Trueordering viaassert_has_calls. Good coverage of the happy path.
25-39: Exercise the finally-path to guarantee end marker on exceptionsThis test proves
<-- line …is emitted even when an exception is raised inside the context. Keep.README.md (4)
11-12: Align features with sequential processing modelDocument sequential per-file diagram processing to match the code, resolving earlier concurrency/ordering concerns.
49-54: Clarify concurrency flag status and discovery semanticsMark
--concurrencyas reserved and explain discovery rules. This sets correct user expectations.
61-66: Define start/end line-number semantics explicitlyState that start = first content line, end = closing fence. This matches
Diagram.line_start/line_end.
74-78: Specify stdout/stderr streams for markers and CLI errorsDocumenting stream behaviour helps CI/log consumers. Good addition.
tests/integration/test_cli_behavior.py (1)
131-181: Integration test robustly verifies per‑diagram schema and line markersThe test asserts exact markers once and ordered at concurrency=1; it also checks single occurrence at concurrency=2. This locks in the CLI contract without depending on ordering under higher concurrency.
nixie/unittests/test_parse_blocks.py (3)
19-22: Validate parsing across fence whitespace variantsAsserting
sourceandschemaunder minor formatting variations increases confidence inBLOCK_REand_extract_schema.
27-31: Strengthen multi-block assertions with positionsAsserting
line_start/line_endfor both diagrams fully validates positional metadata.
44-67: Cover empty, whitespace-only, and comment‑prefixed blocksThese cases validate
UNKNOWN_SCHEMAand comment skipping semantics. Keep.
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
nixie/cli.py (3)
469-479: Flush file boundary markers to keep output strictly bracketedEmit
==>/<==markers withflush=Trueto preserve ordering relative to per‑diagram markers and stderr in CI logs and TTYs.Apply this diff:
- print(f"==> {path}") + print(f"==> {path}", flush=True) try: success = await check_file(path, cfg_path) except Exception as exc: # noqa: BLE001 pragma: no cover - unexpected # Catch unexpected errors so the CLI can continue processing. print(f"Validation task raised an exception: {exc}") success = False if not success: all_success = False - print(f"<== {path}") + print(f"<== {path}", flush=True)
343-343: Write .mmd using explicit UTF‑8 encodingAvoid locale‑dependent defaults when writing source files that can include non‑ASCII content.
Apply this diff:
- mmd.write_text(block) + mmd.write_text(block, encoding="utf-8")
421-424: Replace invalid isinstance union with a tuple of typesFound one occurrence that raises a TypeError at runtime. Update as follows:
• nixie/cli.py:422
- if isinstance(exc, KeyboardInterrupt | SystemExit): + if isinstance(exc, (KeyboardInterrupt, SystemExit)): raise
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- Jira integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
README.md(3 hunks)nixie/cli.py(9 hunks)nixie/unittests/test_render_diagram.py(3 hunks)nixie/unittests/test_verbose.py(5 hunks)tests/integration/conftest.py(0 hunks)tests/integration/test_cli_behavior.py(4 hunks)tests/integration/test_gitignore_paths.py(2 hunks)tests/integration/test_no_args.py(2 hunks)
💤 Files with no reviewable changes (1)
- tests/integration/conftest.py
🧰 Additional context used
📓 Path-based instructions (6)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Python changes must pass tests (unit and behavioral) before completion/commit
Python code must pass lint checks (make lint)
Python code must adhere to formatting standards (make check-fmt/make fmt)
Python code must pass type checking (make typecheck)
Follow core Python 3.13 style conventions per .rules/python-00.md
Apply best practices for context managers per .rules/python-context-managers.md
Follow generator and iterator patterns per .rules/python-generators.md
Follow function return conventions per .rules/python-return.md
Apply Python typing best practices per .rules/python-typing.md
**/*.py: Name Python files in snake_case (e.g., http_client.py, task_queue.py)
Classes must use PascalCase
Variables and functions must use snake_case
Module-level constants use UPPER_SNAKE_CASE
Prefix non-exported helpers or internal APIs with a single underscore
Use typing everywhere; maintain full static type coverage with Pyright
Use TypedDict or @DataClass for structured data; prefer @DataClass(slots=True) for internal-only
Avoid Any; use Unknown, generics, or cast() with justification if Any is used
Be explicit with return types for all public functions and class methods (e.g., -> None, -> str)
Favor immutability (prefer tuples to lists; use frozendict or types.MappingProxyType where appropriate)
Use# pyright: ignoresparingly and include an explanation when used
Avoid side effects at import time; modules should not modify global state or perform actions on import
Never hardcode secrets in source code
Write NumPy-style docstrings for public functions, classes, and modules
Add inline comments to explain non-obvious logic or decisions
**/*.py: Use context managers to encapsulate setup/teardown for resources (files, locks, connections) instead of manual management
Use @contextmanager from contextlib for straightforward, linear setup/teardown without persistent internal state
Implement a class-based context manager (enter/exit) when there is internal sta...
Files:
nixie/unittests/test_render_diagram.pytests/integration/test_gitignore_paths.pynixie/unittests/test_verbose.pytests/integration/test_no_args.pynixie/cli.pytests/integration/test_cli_behavior.py
⚙️ CodeRabbit configuration file
**/*.py: - Keep cyclomatic complexity ≤ 12
- Follow single responsibility and CQRS (command/query segregation)
- Docstrings must follow the
numpystyle guide. Use a single-line summary for private functions and methods, and full structured docs for all public interfaces.- Move conditionals with >2 branches to predicate/helper functions
- Avoid
eval,exec,pickle, monkey-patching,ctypes, unsafe shell- Every module must begin with a triple-quoted docstring explaining its purpose, utility, and usage, including example calls if appropriate.
- Comments and docs must follow en-GB-oxendict (-ize / -our) spelling and grammar
- Lint suppressions:
- Blanket
# noqa, file-level skips, and categories are forbidden- Only narrow in-line disables (
# noqa: XYZ) are permitted, and must be accompanied byFIXME:or a ticket link, and used only as a last resort.- Use
pytestfixtures for shared setup (conftest.pyorfixtures/)- Replace duplicate tests with
@pytest.mark.parametrize- Prefer
pytest-mockorunittest.mockfor stubs/mocks- Use
assert …, "message"over bare asserts- Reflect all API/behaviour changes in
docs/and update roadmap on completion- Files must not exceed 400 logical lines:
- Decompose large modules into subpackages
- Split large
match/caseor dispatch tables by domain and collocate with targets if appropriate- Move bulky data (fixtures, templates) to external files for parsing at runtime
- Mutable defaults and shadowed built-ins are forbidden
- All code must have clear type hints using modern style (
A | B,list[str],class Foo[A]:,type Bar = int, etc.), with ABC imports drawn from the correct stdlib module.
Files:
nixie/unittests/test_render_diagram.pytests/integration/test_gitignore_paths.pynixie/unittests/test_verbose.pytests/integration/test_no_args.pynixie/cli.pytests/integration/test_cli_behavior.py
**/unittests/test_*.py
📄 CodeRabbit inference engine (.rules/python-00.md)
Colocate unit tests with code in an unittests/ subdirectory using files prefixed with test_
Files:
nixie/unittests/test_render_diagram.pynixie/unittests/test_verbose.py
{**/unittests/test_*.py,tests/integration/test_*.py}
📄 CodeRabbit inference engine (.rules/python-00.md)
{**/unittests/test_*.py,tests/integration/test_*.py}: Use pytest idioms: prefer fixtures, parametrize broadly, avoid unnecessary mocks
Group related tests using classes with method names prefixed by test_
Write tests from a user's perspective; test public behaviour, not internals
Avoid excessive mocking; use doubles only for external services or non-deterministic behaviour
Files:
nixie/unittests/test_render_diagram.pytests/integration/test_gitignore_paths.pynixie/unittests/test_verbose.pytests/integration/test_no_args.pytests/integration/test_cli_behavior.py
tests/integration/test_*.py
📄 CodeRabbit inference engine (.rules/python-00.md)
Place integration tests under tests/integration/ with files prefixed with test_
Files:
tests/integration/test_gitignore_paths.pytests/integration/test_no_args.pytests/integration/test_cli_behavior.py
**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
**/*.md: Markdown files must pass markdown linting (make markdownlint)
Markdown files containing Mermaid diagrams must pass nixie validation (make nixie)
Files:
README.md
⚙️ CodeRabbit configuration file
**/*.md: * Avoid 2nd person or 1st person pronouns ("I", "you", "we")
- Use en-GB-oxendict (-ize / -our) spelling and grammar
- Headings must not be wrapped.
- Documents must start with a level 1 heading
- Headings must correctly increase or decrease by no more than one level at a time
- Use GitHub-flavoured Markdown style for footnotes and endnotes.
- Numbered footnotes must be numbered by order of appearance in the document.
Files:
README.md
**/README.md
📄 CodeRabbit inference engine (.rules/python-00.md)
Colocate README.md near reusable packages and include usage examples
Files:
README.md
🔍 Remote MCP Ref
Relevant facts for review (concise):
- New Diagram dataclass added (slots, frozen) with fields: source (str), line_start (int), line_end (int), schema (str).
- parse_blocks now returns list[Diagram], computes line_start/line_end using precomputed newline offsets + bisect to avoid O(N²), and derives schema via _extract_schema. Tests updated to assert positional metadata.
- _extract_schema skips blank lines and lines starting with "%%", returns first token or UNKNOWN_SCHEMA fallback. UNKNOWN_SCHEMA exported.
- diagram_markers context manager prints start ("--> line {line_start}: {schema}") before processing and end ("<-- line {line_end}: {schema}") on exit (flush=True), ensuring end marker on exceptions. Unit tests verify both markers and exception behavior.
- check_file and overall CLI processing changed to process diagrams sequentially per file (for-loop) rather than concurrently; concurrency primitives and CLI option removed. Integration test added to assert per-diagram markers appear exactly once and in ascending order for a file with multiple diagrams.
Tools/sources used:
- Ref (repository docs/search results) — results mentioning the PR diffs and code changes above.
🔇 Additional comments (9)
tests/integration/test_gitignore_paths.py (1)
30-31: Align integration tests with the new main(paths) APICalls to
main([Path(".")])read cleanly and match the sequential rendering design. Assertions remain correct.Also applies to: 50-51
nixie/unittests/test_verbose.py (1)
51-52: Update render_block invocations to match semaphore‑free signatureThe changes reflect the public API and the tests continue to assert the intended logging/error behaviours.
Also applies to: 136-137, 165-166, 189-189, 215-216
nixie/unittests/test_render_diagram.py (1)
42-43: Conform tests to updated helpers and allowed‑executable guardThe adjustments to
_render_diagram/_run_mermaid_clisignatures and the guard assertion look correct.Also applies to: 76-77, 90-91
tests/integration/test_no_args.py (1)
30-33: Remove unused concurrency parameter from test doublesThe local
fake_mainstubs now matchmain(paths)precisely; exit semantics and captured paths assertions remain sound.Also applies to: 56-60
tests/integration/test_cli_behavior.py (2)
86-86: Default to sequential main() for deterministic output — good changeDrop the explicit concurrency parameter and rely on the CLI’s sequential default to stabilise marker ordering. This directly removes prior flakiness risk around interleaved output.
113-113: Retain stable file-boundary ordering — good changeInvoke main() without a concurrency argument to keep file boundary markers ordered and repeatable.
README.md (3)
11-12: Document sequential per‑file processing for stable, bracketed output — goodState the guarantee explicitly; aligns the docs with the CLI refactor and the new tests.
69-71: Keep the example concise and aligned with semantics — goodThe example matches the “start = first content line inside the fence, end = closing fence” rule and shows the echoed schema.
74-78: Clarify stdout/stderr streams — goodExplicitly document stream usage to aid CI/log consumers. This mirrors the actual behaviour and reduces surprises when stderr is treated specially.
|
@coderabbitai pause |
✅ Actions performedReviews paused. |
Ensure per-diagram markers flush immediately so they bracket stderr reliably and add a unit test to cover the behavior.
- Resolve conflicts in README, nixie/cli.py, and tests - Drop concurrency scaffolding; keep sequential, bracketed output - Add --no-sandbox flag and pass through to Puppeteer config - Reconcile tests to new main(paths, *, no_sandbox=False) signature - Add minimal local pathspec shim for offline testing Preserves branch intent (schema + line markers) while incorporating main’s improvements. CI with real pathspec should ignore the shim.
9fac5c6 to
9d20227
Compare
1 participant
Summary
Testing
make fmtmake check-fmtmake lintmake typecheckmake testmake markdownlint(fails: Missing link/image reference definitions in .rules/python-pyproject.md)/root/.bun/bin/markdownlint-cli2 README.mdmake nixiehttps://chatgpt.com/codex/tasks/task_e_68a50b770ca08322add66a36388a407b
Summary by Sourcery
Print schema names and line number markers around each Mermaid diagram in the CLI, update parsing to capture metadata, document the new markers in the README, and include an integration test to verify the behavior.
New Features:
Enhancements:
Documentation:
Tests: