Conversation
Reviewer's GuideThis PR consolidates the temporary file creation and Mermaid CLI invocation into a single Class diagram for refactored rendering helpersclassDiagram
class render_block {
+async def render_block(block, tmpdir, cfg_path, path, idx, semaphore, timeout=30.0) bool
}
class _render_diagram {
+async def _render_diagram(block, tmpdir, cfg_path, path, idx, sem, timeout) None
}
render_block --> _render_diagram : delegates rendering
Flow diagram for unified diagram rendering processflowchart TD
A[Start render_block] --> B[Call _render_diagram]
B --> C[Write Mermaid block to .mmd file]
C --> D[Invoke mermaid-cli to render SVG]
D --> E{Success?}
E -- Yes --> F[Return True]
E -- No --> G[Handle error, print to stderr]
G --> H[Return False]
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. Summary by CodeRabbit
WalkthroughUpdate exception handling in Changes
Sequence Diagram(s)No sequence diagram generated as the changes do not introduce or alter control flow or feature interactions. Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 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 comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @leynos - I've reviewed your changes and found some issues that need to be addressed.
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `nixie/cli.py:140` </location>
<code_context>
- f"{format_cli_error(stderr.decode('utf-8', errors='replace'))}"
- )
- raise RuntimeError(error_message)
+ raise RuntimeError(format_cli_error(stderr.decode("utf-8", errors="replace")))
</code_context>
<issue_to_address>
Loss of context in error message compared to previous implementation.
Including the command, file, and diagram index in the error message would improve debugging and troubleshooting.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
raise RuntimeError(format_cli_error(stderr.decode("utf-8", errors="replace")))
=======
error_message = (
f"Command '{cmd}' failed for file '{path}' (diagram index {idx}):\n"
f"{format_cli_error(stderr.decode('utf-8', errors='replace'))}"
)
raise RuntimeError(error_message)
>>>>>>> REPLACE
</suggested_fix>
### Comment 2
<location> `nixie/cli.py:110` </location>
<code_context>
-async def _run_mermaid_cli(
- cmd: list[str],
- semaphore: asyncio.Semaphore,
+async def _render_diagram(
+ block: str,
+ tmpdir: Path,
</code_context>
<issue_to_address>
Consider splitting file writing and CLI invocation into separate helpers to improve single-responsibility and testability.
Consider restoring single-responsibility and testability by pulling file-I/O and CLI invocation back out of `_render_diagram`. For example:
```python
def _prepare_diagram_paths(block: str, tmpdir: Path, path: Path, idx: int) -> tuple[Path, Path]:
mmd = tmpdir / f"{path.stem}_{idx}.mmd"
svg = mmd.with_suffix(".svg")
mmd.write_text(block)
return mmd, svg
async def _invoke_mermaid(
cmd: list[str],
sem: asyncio.Semaphore,
path: Path,
idx: int,
timeout: float,
) -> None:
logging.getLogger(__name__).info(shlex.join(cmd))
async with sem:
proc = await asyncio.create_subprocess_exec(
*cmd,
stdout=asyncio.subprocess.PIPE,
stderr=asyncio.subprocess.PIPE,
)
success, stderr = await wait_for_proc(proc, path, idx, timeout)
if not success:
raise RuntimeError(format_cli_error(stderr.decode("utf-8", errors="replace")))
```
Then simplify `_render_diagram` to just:
```python
async def _render_diagram(
block: str,
tmpdir: Path,
cfg_path: Path,
path: Path,
idx: int,
sem: asyncio.Semaphore,
timeout: float,
) -> None:
mmd, svg = _prepare_diagram_paths(block, tmpdir, path, idx)
cmd = get_mmdc_cmd(mmd, svg, cfg_path)
await _invoke_mermaid(cmd, sem, path, idx, timeout)
```
This keeps each helper tiny (and easy to test), reduces the overall signature surface in each function, and preserves exactly the same behavior.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![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
🔭 Outside diff range comments (1)
nixie/cli.py (1)
152-174: Simplified error handling improves maintainability.The updated
render_blockfunction properly delegates to_render_diagramand handles exceptions appropriately. The error reporting is clear and user-friendly.However, address the broad exception handling as flagged by static analysis.
The broad
Exceptioncatch on line 169 violates best practices. Replace with specific exception types or document why broad catching is necessary:- except Exception as exc: # pragma: no cover - unexpected + except Exception as exc: # pragma: no cover - FIXME: narrow exception types
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
nixie/cli.py(4 hunks)nixie/unittests/test_render_diagram.py(3 hunks)nixie/unittests/test_write_diagram_file.py(0 hunks)
💤 Files with no reviewable changes (1)
- nixie/unittests/test_write_diagram_file.py
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit Inference Engine (AGENTS.md)
For Python files: Pass all relevant unit and behavioral tests, pass lint checks (
ruff check), adhere to formatting standards (ruff format), and pass type checking (pyright).
**/*.py: Use snake_case.py for file names, naming files for their contents (e.g., http_client.py, task_queue.py)
Use PascalCase for class names in Python files
Use snake_case for variable and function names in Python files
Use UPPER_SNAKE_CASE for module-level constants in Python files
Prefix private/internal helpers or APIs with a single underscore (_) in Python files
Use typing everywhere and maintain full static type coverage in Python files; use Pyright for type-checking
Use TypedDict or Dataclass for structured data where appropriate; for internal-only usage, prefer @DataClass(slots=True)
Avoid Any in type annotations; use Unknown, generics, or cast() when necessary, and always document why Any is acceptable if used
Be explicit with return types in public functions and class methods (e.g., use -> None, -> str, etc.)
Favor immutability in Python files; prefer tuples over lists, and frozendict or types.MappingProxyType where appropriate
Enable Ruff for linting and formatting; use Ruff to lint for performance, security, consistency, and style issues, and let Ruff handle whitespace and formatting entirely
Enforce strict mode in Pyright and treat all Pyright warnings as CI errors; use # pyright: ignore sparingly and with explanation
Avoid side effects at import time in Python modules; modules should not modify global state or perform actions on import
Use docstrings to document public functions, classes, and modules using NumPy format
Explain tricky code with inline comments for non-obvious logic or decisions in Python files
**/*.py: Use context managers to encapsulate setup and teardown logic cleanly and safely, especially for resource management (files, locks, connections, etc.), instead of manual try/finally blocks.
Use@contextmanagerfromcontextlibfor straightforward proce...
Files:
nixie/unittests/test_render_diagram.pynixie/cli.py
⚙️ CodeRabbit Configuration File
**/*.py: How about the following?- Keep cyclomatic complexity ≤ 12 - Follow single responsibility and CQRS (command/query segregation) - Docstrings must follow the `numpy` style 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 by `FIXME:` or a ticket link, and used only as a last resort. - Use `pytest` fixtures for shared setup (`conftest.py` or `fixtures/`) - Replace duplicate tests with `@pytest.mark.parametrize` - Prefer `pytest-mock` or `unittest.mock` for 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/case` or 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.pynixie/cli.py
**/unittests/test_*.py
📄 CodeRabbit Inference Engine (.rules/python-00.mdc)
Colocate unit tests with code using a unittests subdirectory and a test_ prefix for test files
Files:
nixie/unittests/test_render_diagram.py
**/test_*.py
📄 CodeRabbit Inference Engine (.rules/python-00.mdc)
**/test_*.py: Use pytest idioms in test files: prefer fixtures over setup/teardown methods, parametrize broadly, and avoid unnecessary mocks
Group related tests using class with method names prefixed by test_ in test files
Files:
nixie/unittests/test_render_diagram.py
🧬 Code Graph Analysis (1)
nixie/unittests/test_render_diagram.py (1)
nixie/cli.py (3)
cli(262-269)_render_diagram(110-140)get_mmdc_cmd(62-79)
🪛 Ruff (0.12.2)
nixie/unittests/test_render_diagram.py
49-49: Trailing comma missing
Add trailing comma
(COM812)
nixie/cli.py
117-117: Async function definition with a timeout parameter
(ASYNC109)
163-164: Use format specifiers instead of percent format
Replace with format specifiers
(UP031)
169-169: Do not catch blind exception: Exception
(BLE001)
🔇 Additional comments (5)
nixie/unittests/test_render_diagram.py (3)
10-10: LGTM on import update.The import correctly reflects the refactored API by importing
_render_diagraminstead of the previous separate functions.
14-44: Test correctly validates the consolidated rendering workflow.The test properly validates that
_render_diagramwrites the Mermaid block to a file and logs the CLI command. The dynamic construction of expected file paths and command verification is well-implemented.
48-74: Test correctly validates error handling in consolidated workflow.The failure test properly verifies that
_render_diagramraisesRuntimeErrorwith formatted CLI error output when the subprocess fails. The simplified error message assertions align with the refactored error handling.nixie/cli.py (2)
6-6: LGTM on subprocess import addition.The explicit import of
asyncio.subprocessis appropriate for the subprocess management in_render_diagram.
110-141: Excellent consolidation of rendering workflow.The
_render_diagramfunction effectively consolidates file writing and CLI invocation, reducing parameter surface area and centralising the subprocess management. The function handles file creation, command construction, logging, and error formatting in a cohesive manner.Note: The static analysis warning about async timeout parameters (ASYNC109) can be safely ignored here as the timeout is properly used with
asyncio.wait_for()in thewait_for_procfunction.
Consolidate temporary file creation and mermaid CLI invocation into _render_diagram, simplifying render_block. Update tests accordingly.
leynos
force-pushed
the
codex/refactor-diagram-rendering-functions
branch
from
ca0214c to
5656e78
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (3)
nixie/cli.py (3)
109-117: Catchasyncio.TimeoutError(or both) to ensure timeout handling and process cleanup.
asyncio.wait_forraisesasyncio.TimeoutError. Catching only built-inTimeoutErrorrisks missing the timeout on some Python versions, leaving the spawned process alive and skipping the intendedkill()/wait()path.Apply this diff:
- except TimeoutError: + except (asyncio.TimeoutError, TimeoutError):
166-174: Hold the semaphore for the entire CLI execution to actually cap concurrency.Releasing the semaphore before
wait_for_procallows unlimited concurrent CLI processes; it only gates process creation, not execution. Gate the full lifecycle.Apply this diff:
- async with sem: - proc = await asyncio.create_subprocess_exec( # nosemgrep: python.lang.security.audit.dangerous-asyncio-create-exec-audit - *cmd, - stdout=asyncio.subprocess.PIPE, - stderr=asyncio.subprocess.PIPE, - ) - - success, stderr = await wait_for_proc(proc, path, idx, timeout) + async with sem: + proc = await asyncio.create_subprocess_exec( # nosemgrep: python.lang.security.audit.dangerous-asyncio-create-exec-audit + *cmd, + stdout=asyncio.subprocess.PIPE, + stderr=asyncio.subprocess.PIPE, + ) + success, stderr = await wait_for_proc(proc, path, idx, timeout)
268-271: Apply the same BLE001 resolution to the top-level catch inmain.This is another intentional boundary; either suppress narrowly with rationale or narrow the exception set. Keep behaviour consistent with
render_block.Example (keep and justify):
- except Exception as exc: # pragma: no cover - unexpected + except Exception as exc: # pragma: no cover - unexpected # noqa: BLE001 - CLI boundary: fail file, keep process running print(f"Validation task raised an exception: {exc}") success = False
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
nixie/cli.py(2 hunks)nixie/unittests/test_render_diagram.py(0 hunks)
💤 Files with no reviewable changes (1)
- nixie/unittests/test_render_diagram.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit Inference Engine (AGENTS.md)
For Python files: Pass all relevant unit and behavioral tests, pass lint checks (
ruff check), adhere to formatting standards (ruff format), and pass type checking (pyright).
**/*.py: Use snake_case.py for file names, naming files for their contents (e.g., http_client.py, task_queue.py)
Use PascalCase for class names in Python files
Use snake_case for variable and function names in Python files
Use UPPER_SNAKE_CASE for module-level constants in Python files
Prefix private/internal helpers or APIs with a single underscore (_) in Python files
Use typing everywhere and maintain full static type coverage in Python files; use Pyright for type-checking
Use TypedDict or Dataclass for structured data where appropriate; for internal-only usage, prefer @DataClass(slots=True)
Avoid Any in type annotations; use Unknown, generics, or cast() when necessary, and always document why Any is acceptable if used
Be explicit with return types in public functions and class methods (e.g., use -> None, -> str, etc.)
Favor immutability in Python files; prefer tuples over lists, and frozendict or types.MappingProxyType where appropriate
Enable Ruff for linting and formatting; use Ruff to lint for performance, security, consistency, and style issues, and let Ruff handle whitespace and formatting entirely
Enforce strict mode in Pyright and treat all Pyright warnings as CI errors; use # pyright: ignore sparingly and with explanation
Avoid side effects at import time in Python modules; modules should not modify global state or perform actions on import
Use docstrings to document public functions, classes, and modules using NumPy format
Explain tricky code with inline comments for non-obvious logic or decisions in Python files
**/*.py: Use context managers to encapsulate setup and teardown logic cleanly and safely, especially for resource management (files, locks, connections, etc.), instead of manual try/finally blocks.
Use@contextmanagerfromcontextlibfor straightforward proce...
Files:
nixie/cli.py
⚙️ CodeRabbit Configuration File
**/*.py: How about the following?- Keep cyclomatic complexity ≤ 12 - Follow single responsibility and CQRS (command/query segregation) - Docstrings must follow the `numpy` style 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 by `FIXME:` or a ticket link, and used only as a last resort. - Use `pytest` fixtures for shared setup (`conftest.py` or `fixtures/`) - Replace duplicate tests with `@pytest.mark.parametrize` - Prefer `pytest-mock` or `unittest.mock` for 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/case` or 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
🪛 Ruff (0.12.2)
nixie/cli.py
220-220: Do not catch blind exception: Exception
(BLE001)
🔇 Additional comments (1)
nixie/cli.py (1)
213-217: Use f-string for error message — LGTM.Maintain this style for consistency with modern Python and Ruff’s UP031 rule.
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
nixie/cli.py (1)
165-173: Hold the semaphore for the full subprocess lifecycleEnforce the concurrency limit across the entire CLI execution. As written, the semaphore is released immediately after spawn, so the number of concurrently running
mmdcprocesses is effectively unbounded.- async with sem: - proc = await asyncio.create_subprocess_exec( # nosemgrep: python.lang.security.audit.dangerous-asyncio-create-exec-audit - *cmd, - stdout=asyncio.subprocess.PIPE, - stderr=asyncio.subprocess.PIPE, - ) - - success, stderr = await wait_for_proc(proc, path, idx, timeout) + async with sem: + proc = await asyncio.create_subprocess_exec( # nosemgrep: python.lang.security.audit.dangerous-asyncio-create-exec-audit + *cmd, + stdout=asyncio.subprocess.PIPE, + stderr=asyncio.subprocess.PIPE, + ) + success, stderr = await wait_for_proc(proc, path, idx, timeout)
♻️ Duplicate comments (1)
nixie/cli.py (1)
265-267: Either justify or remove blanketexcept Exceptioninmain(Ruff BLE001)Treat this as a deliberate CLI boundary or remove it. If keeping it to ensure robust CLI UX, add a narrow suppression with rationale to satisfy linting.
- except Exception as exc: # pragma: no cover - unexpected + except Exception as exc: # pragma: no cover - unexpected # noqa: BLE001 - CLI boundary: log-and-continue by design print(f"Validation task raised an exception: {exc}")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
nixie/cli.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit Inference Engine (AGENTS.md)
For Python files: Pass all relevant unit and behavioral tests, pass lint checks (
ruff check), adhere to formatting standards (ruff format), and pass type checking (pyright).
**/*.py: Use snake_case.py for file names, naming files for their contents (e.g., http_client.py, task_queue.py)
Use PascalCase for class names in Python files
Use snake_case for variable and function names in Python files
Use UPPER_SNAKE_CASE for module-level constants in Python files
Prefix private/internal helpers or APIs with a single underscore (_) in Python files
Use typing everywhere and maintain full static type coverage in Python files; use Pyright for type-checking
Use TypedDict or Dataclass for structured data where appropriate; for internal-only usage, prefer @DataClass(slots=True)
Avoid Any in type annotations; use Unknown, generics, or cast() when necessary, and always document why Any is acceptable if used
Be explicit with return types in public functions and class methods (e.g., use -> None, -> str, etc.)
Favor immutability in Python files; prefer tuples over lists, and frozendict or types.MappingProxyType where appropriate
Enable Ruff for linting and formatting; use Ruff to lint for performance, security, consistency, and style issues, and let Ruff handle whitespace and formatting entirely
Enforce strict mode in Pyright and treat all Pyright warnings as CI errors; use # pyright: ignore sparingly and with explanation
Avoid side effects at import time in Python modules; modules should not modify global state or perform actions on import
Use docstrings to document public functions, classes, and modules using NumPy format
Explain tricky code with inline comments for non-obvious logic or decisions in Python files
**/*.py: Use context managers to encapsulate setup and teardown logic cleanly and safely, especially for resource management (files, locks, connections, etc.), instead of manual try/finally blocks.
Use@contextmanagerfromcontextlibfor straightforward proce...
Files:
nixie/cli.py
⚙️ CodeRabbit Configuration File
**/*.py: How about the following?- Keep cyclomatic complexity ≤ 12 - Follow single responsibility and CQRS (command/query segregation) - Docstrings must follow the `numpy` style 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 by `FIXME:` or a ticket link, and used only as a last resort. - Use `pytest` fixtures for shared setup (`conftest.py` or `fixtures/`) - Replace duplicate tests with `@pytest.mark.parametrize` - Prefer `pytest-mock` or `unittest.mock` for 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/case` or 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
🔇 Additional comments (1)
nixie/cli.py (1)
214-214: Good: modernise message formatting with an f-stringAdopt f-strings for the CLI-not-found error. This aligns with Ruff UP031 and improves readability.
| try: | ||
| _, stderr = await asyncio.wait_for(proc.communicate(), timeout) | ||
| except asyncio.TimeoutError: | ||
| except TimeoutError: |
There was a problem hiding this comment.
Fix timeout exception class to catch asyncio timeouts
Catch asyncio.TimeoutError (raised by asyncio.wait_for), not the built-in TimeoutError. The current clause will fail to intercept timeouts, changing behaviour and leaking processes until the outer scope handles the exception.
- except TimeoutError:
+ except asyncio.TimeoutError:📝 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.
| except TimeoutError: | |
| except asyncio.TimeoutError: |
🤖 Prompt for AI Agents
In nixie/cli.py at line 110, replace the built-in TimeoutError exception with
asyncio.TimeoutError to correctly catch timeouts raised by asyncio.wait_for.
Import asyncio if not already imported, and update the except clause to except
asyncio.TimeoutError to prevent unhandled timeout exceptions and potential
process leaks.
There was a problem hiding this comment.
🧩 Analysis chain
@leynos I'll create a GitHub issue for the timeout exception fix.
🏁 Script executed:
#!/bin/bash
gh issue create \
--title "Fix timeout exception handling in wait_for_proc function" \
--body "## Description
The \`wait_for_proc\` function in \`nixie/cli.py\` currently catches the built-in \`TimeoutError\` instead of \`asyncio.TimeoutError\`. This prevents proper handling of timeouts raised by \`asyncio.wait_for\`, potentially leading to unhandled exceptions and process leaks.
## Expected Behaviour
The function should catch \`asyncio.TimeoutError\` to properly handle timeouts from \`asyncio.wait_for\`.
## Current Behaviour
The function catches the built-in \`TimeoutError\`, which will not intercept timeouts from \`asyncio.wait_for\`.
## Fix
Change line 110 from:
\`\`\`python
except TimeoutError:
\`\`\`
to:
\`\`\`python
except asyncio.TimeoutError:
\`\`\`
## References
- PR: https://github.com/leynos/nixie/pull/13
- Comment: https://github.com/leynos/nixie/pull/13#discussion_r2261636969
Requested by: @leynos" \
--assignee leynos
Length of output: 959
@leynos I've successfully created GitHub issue #15 for the timeout exception handling fix. The issue includes:
- Clear description of the problem with the incorrect exception type
- Expected vs current behaviour
- The specific code change needed
- References to this PR and comment thread
- Assignment to you as requested
You can view the issue at: #15
🧬 Code Graph Analysis Results
Relevant snippet from nixie/unittests/test_get_mmdc_cmd.py (lines 22-25)
def which(cmd: str) -> str | None:
if cmd == "bun":
return "/usr/bin/bun"
return NoneThis snippet defines a mock which function that returns a path for the "bun" command and None otherwise. It is relevant because the nixie/cli.py file uses shutil.which to detect the availability of CLI executables (mmdc, bun, npx) in the get_mmdc_cmd function. This mock could be part of tests that simulate the presence or absence of these executables.
1 participant
Summary
_render_diagramrender_blockto delegate rendering logic and report failures_render_diagramsuccess and error pathsTesting
make fmtmake lintmake typecheckmake testhttps://chatgpt.com/codex/tasks/task_e_6894edfc5bdc8322ba7765033e341b37
Summary by Sourcery
Unify file writing and Mermaid CLI invocation into a single helper, simplify render_block to delegate rendering and handle errors, and update unit tests to cover the new rendering workflow.
Enhancements:
Tests: