Print celebratory message on validation success - #27
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR enhances the CLI to print a celebratory message when all diagrams validate successfully and adds an integration test to cover this behavior. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary by CodeRabbit
WalkthroughAdd a module-level Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant CLI as nixie/cli.py
participant V as Validator
U->>CLI: Run command with file(s)
loop For each Markdown path
CLI->>V: Validate path
V-->>CLI: Result (success/failure)
alt all_success still True
CLI-->>U: Print SUCCESS_BANNER (stdout, flush)
else Failure encountered
CLI-->>U: Do not print banner for this/remaining paths
end
end
CLI-->>U: Exit (0 if all succeeded, non‑zero otherwise)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
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}
![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
📜 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 (2)
nixie/cli.py(1 hunks)tests/integration/test_cli_behavior.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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/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/cli.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_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.py
🔇 Additional comments (1)
nixie/cli.py (1)
534-536: LGTM: Print celebratory banner once at the end.Emits a clear, user-facing success indicator without altering control flow.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
nixie/cli.py (1)
537-538: LGTM — emit banner once after overall success with immediate flush.This implements the earlier suggestion and prevents duplicate banners; stdout + flush=True is appropriate.
tests/integration/test_cli_behavior.py (1)
8-8: LGTM — import the module constant to prevent drift.Ties the test to the single source of truth for the banner text.
📜 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 (2)
nixie/cli.py(3 hunks)tests/integration/test_cli_behavior.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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/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/cli.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_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.py
🧬 Code graph analysis (1)
tests/integration/test_cli_behavior.py (1)
nixie/cli.py (2)
cli(573-590)main(517-539)
🔍 Remote MCP
Here’s a concise set of concrete facts to inform your review:
- The
typing.Finalqualifier (PEP 591) was added in Python 3.8. UsingSUCCESS_BANNER: Final[str] = "…"marks it as a constant that type-checkers will forbid from being reassigned or overridden, but imposes no runtime behavior change (docs.python.org). - The
print(..., flush=True)parameter (available since Python 3.3) forces an immediate flush of the output buffer, ensuring the banner is emitted to stdout without waiting on buffer-full or program termination (stackoverflow.com). - The new integration test uses pytest’s
capsysfixture to capture standard output and asserts thatSUCCESS_BANNERappears exactly once when the exit code is 0, and never when it’s nonzero (pertests/integration/test_cli_behavior.py).
🔇 Additional comments (1)
tests/integration/test_cli_behavior.py (1)
102-106: LGTM — assert exact banner emission count.Correctly enforces one banner on success and none on failure.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 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 (1)
nixie/cli.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.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/cli.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.py
🔍 Remote MCP
Here’s a concise set of concrete facts to inform your review:
- The
typing.Finalqualifier (PEP 591) was added in Python 3.8. UsingSUCCESS_BANNER: Final[str] = "…"marks it as a constant that type-checkers will forbid from being reassigned or overridden, but imposes no runtime behavior change (docs.python.org). - The
print(..., flush=True)parameter (available since Python 3.3) forces an immediate flush of the output buffer, ensuring the banner is emitted to stdout without waiting on buffer-full or program termination (stackoverflow.com). - The new integration test uses pytest’s
capsysfixture to capture standard output and asserts thatSUCCESS_BANNERappears exactly once when the exit code is 0, and never when it’s nonzero (pertests/integration/test_cli_behavior.py).
🔇 Additional comments (2)
nixie/cli.py (2)
97-98: Promote banner to a constant with typ.Final — LGTM.Keep the constant in UPPER_SNAKE_CASE and annotated as typ.Final to match project style. No import side-effects introduced.
536-538: Print banner once at run end with flush — LGTM.Preserve exit semantics and emit to stdout deterministically. Using flush=True is appropriate for CI logs.
| if all_success: | ||
| print(SUCCESS_BANNER, flush=True) | ||
| return 0 if all_success else 1 |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Clarify banner semantics for “no diagrams” runs.
Decide whether to suppress the success banner when zero diagrams are found across all files. Current logic prints the banner because all_success stays True even when nothing was validated. If the intended behaviour is “print only when at least one diagram validated”, gate on a “saw_diagram” flag aggregated across files and assert this in tests.
🤖 Prompt for AI Agents
In nixie/cli.py around lines 536 to 538, the success banner is printed whenever
all_success is True even if no diagrams were processed; add an aggregated
saw_diagram boolean (set to True when any file yields at least one diagram) and
change the banner print condition to require both all_success and saw_diagram.
Update the per-file processing to flip saw_diagram when diagrams are
encountered, and add/update tests to assert that the banner is printed only when
at least one diagram was validated and suppressed when zero diagrams are found.
1 participant
Summary
Testing
make check-fmtmake lintmake typecheckmake testhttps://chatgpt.com/codex/tasks/task_e_68b1bfaac39c8322b4500f156e3c8ba3
Summary by Sourcery
Print a festive success message after all diagrams validate and add an integration test to ensure it only shows on success
New Features:
Tests: