Skip to content

Implement typed command core with safety - #7

Merged
leynos merged 7 commits into
mainfrom
terragon/implement-typed-command-core-gq70bq
Dec 4, 2025
Merged

Implement typed command core with safety#7
leynos merged 7 commits into
mainfrom
terragon/implement-typed-command-core-gq70bq

Conversation

@leynos

@leynos leynos commented Dec 2, 2025

Copy link
Copy Markdown
Owner

Summary

  • Introduces a typed command core for curated programs via a SafeCmd data object.
  • Adds a factory sh.make that validates programs against the catalogue and builds SafeCmd instances with structured argv and attached project metadata.
  • Improves argv construction: positional args are stringified; keyword args become CLI-style --flag=value entries (underscores in flag names are hyphenated). None values are rejected to catch mistakes early.
  • Exposes SafeCmd and SafeCmdBuilder publicly and wires up a small façade module (cuprum.sh).

Changes

  • New module: cuprum/sh.py
    • SafeCmd dataclass (program, argv, project)
    • SafeCmdBuilder type and make(program, catalogue=...) factory
    • Internal helpers: _stringify_arg, _serialize_kwargs, _coerce_argv
    • argv_with_program property for easy access to full argv including program name
    • Validation against a catalogue; unknown programs raise UnknownProgramError
  • Public API exposure
    • Exported SafeCmd, SafeCmdBuilder, UnknownProgramError, and sh in cuprum/init.py
  • Tests
    • Added unit tests: cuprum/unittests/test_sh.py covering
      • Unknown program rejection
      • SafeCmd metadata population from catalogue
      • Keyword args serialisation to flags
      • Stringification of non-string arguments
      • Custom catalogue usage
    • Extended behaviour/tests to include a safe command builder scenario
  • Documentation
    • Updated docs/cuprum-design.md with implementation notes for the first iteration
    • Updated docs/users-guide.md to describe the Typed command core and provide usage examples
    • Roadmap updated to reflect completed sh.make task

How it works

  • sh.make(program, catalogue=...) validates the provided program exists in the catalogue and returns a builder.
  • The builder, when called as builder(*args, **kwargs), produces a SafeCmd:
    • argv is the positional args stringified plus --flag=value entries for kwargs (normalized keys replace _ with -).
    • argv_with_program prefixes the argv with the program name.
    • project holds the corresponding ProjectSettings from the catalogue for downstream visibility (noise rules, docs links).

Examples

from cuprum import ECHO, sh

cmd_builder = sh.make(ECHO)
cmd = cmd_builder("-n", "hello world", punctuation="!")
print(cmd.argv)  # ('-n', 'hello world', '--punctuation=!')
print(cmd.argv_with_program)  # (ECHO, '-n', 'hello world', '--punctuation=!')
print(cmd.project.noise_rules)  # metadata for downstream loggers

Testing plan

  • Unknown programs are rejected by the builder factory
  • Builder returns SafeCmd with correct program, argv, and project metadata
  • Keyword args serialize to --flag=value and underscores are hyphenated
  • Non-string positional args are stringified in order
  • Custom catalogue can drive project metadata surfaced on SafeCmd
  • Safety and type surface validated via unit tests and feature scenarios

Notes

  • Execution of the commands remains out of scope; SafeCmd is a data object that carries metadata and a structured argv for downstream runtimes.
  • The design ensures an explicit gate through the catalogue, improving safety and predictability of command construction.

📎 Task: https://www.terragonlabs.com/task/853937e5-d75d-4e73-b78c-60c795e47b93

Introduce a new module `sh` providing a typed core for building `SafeCmd` instances from curated `Program` values. The `sh.make` function enforces the curated catalogue allowlist upfront, converts positional and keyword arguments to a structured argv tuple, and attaches project metadata to produced commands. This facilitates safer CLI command assembly, carrying metadata for downstream runtime layers without executing commands at this phase.

Includes:
- `SafeCmd` dataclass to encapsulate program, argv, and project metadata
- `SafeCmdBuilder` callable factory to build parameterized commands
- Validation against the `ProgramCatalogue` with `UnknownProgramError` on unknown programs
- Stringification of arguments with `None` rejection
- Conversion of keyword args to CLI flags `--flag=value` with underscores to hyphens

Also adds unit tests, documentation updates, and BDD features covering this new typed command core.

Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
@sourcery-ai

sourcery-ai Bot commented Dec 2, 2025

Copy link
Copy Markdown
Contributor

Reviewer's Guide

Introduces a new typed SafeCmd command core and sh.make factory for building catalogue-validated command objects with structured argv and attached project metadata, wires these into the public cuprum API, and adds behaviour/docs/tests around safe argv handling and builder usage.

Sequence diagram for sh.make usage and SafeCmd construction

sequenceDiagram
    actor Client
    participant sh as sh_module
    participant Catalogue as ProgramCatalogue
    participant Builder as SafeCmdBuilder
    participant Cmd as SafeCmd

    Client->>sh: make(program, catalogue)
    sh->>Catalogue: lookup(program)
    alt program known
        Catalogue-->>sh: ProgramEntry(program, project)
        sh-->>Client: Builder
        Client->>Builder: __call__(*args, **kwargs)
        Builder->>Builder: _coerce_argv(args, kwargs)
        Builder->>Cmd: construct SafeCmd(program, argv, project)
        Cmd-->>Client: SafeCmd instance
        Client->>Cmd: argv_with_program
        Cmd-->>Client: (program, *argv)
    else program unknown
        Catalogue-->>sh: raise UnknownProgramError
        sh-->>Client: UnknownProgramError
    end
Loading

Class diagram for SafeCmd core and sh.make factory

classDiagram
    class Program {
    }

    class ProjectSettings {
    }

    class ProgramCatalogue {
        +lookup(program: Program) ProgramEntry
    }

    class ProgramEntry {
        +program: Program
        +project: ProjectSettings
    }

    class SafeCmd~Out_co~ {
        +program: Program
        +argv: tuple~str,...~
        +project: ProjectSettings
        +argv_with_program() tuple~str,...~
    }

    class SafeCmd_str {
    }

    class SafeCmdBuilder {
        <<callable>>
        +__call__(*args: object, **kwargs: object) SafeCmd~str~
    }

    class sh_module {
        +_stringify_arg(value: object) str
        +_serialize_kwargs(kwargs: dict~str,object~) tuple~str,...~
        +_coerce_argv(args: tuple~object,...~, kwargs: dict~str,object~) tuple~str,...~
        +make(program: Program, catalogue: ProgramCatalogue) SafeCmdBuilder
    }

    ProgramCatalogue --> ProgramEntry : returns
    ProgramEntry --> Program : has
    ProgramEntry --> ProjectSettings : has

    SafeCmd~Out_co~ --> Program : uses
    SafeCmd~Out_co~ --> ProjectSettings : uses

    SafeCmd_str --> SafeCmd~Out_co~ : specializes

    SafeCmdBuilder --> SafeCmd_str : builds

    sh_module --> ProgramCatalogue : uses
    sh_module --> SafeCmdBuilder : returns
    sh_module --> SafeCmd_str : constructs
    sh_module --> SafeCmd~Out_co~ : constructs
Loading

Flow diagram for argv construction in SafeCmd builder

flowchart TD
    A["Start: builder(*args, **kwargs)"] --> B["Iterate over positional args"]
    B --> C["For each arg: _stringify_arg(arg)"]
    C --> D{"arg is None?"}
    D -->|Yes| E["Raise TypeError: None is not a valid argv element"]
    D -->|No| F["Append str(arg) to positional list"]
    F --> G{"More positional args?"}
    G -->|Yes| C
    G -->|No| H["Iterate over keyword items"]

    H --> I["For each key, value in kwargs"]
    I --> J["Normalize key: key.replace('_','-')"]
    J --> K["_stringify_arg(value)"]
    K --> L{"value is None?"}
    L -->|Yes| E
    L -->|No| M["Build flag '--{normalized_key}={value_str}'"]
    M --> N["Append flag to flags list"]
    N --> O{"More keyword items?"}
    O -->|Yes| I
    O -->|No| P["Combine positional tuple + flags tuple"]
    P --> Q["Return argv tuple of str"]
Loading

File-Level Changes

Change Details Files
Add sh.safe command construction core with typed SafeCmd and builder support
  • Introduce SafeCmd frozen dataclass with program, argv, project and argv_with_program helper
  • Define SafeCmdBuilder callable type and sh.make factory that validates programs against a ProgramCatalogue and raises UnknownProgramError for unknown entries
  • Implement internal helpers to stringify args, serialize kwargs into --flag=value flags with underscore-to-hyphen normalization, and reject None values
cuprum/sh.py
Expose SafeCmd core and sh facade via the top-level cuprum package
  • Re-export SafeCmd and SafeCmdBuilder from cuprum.sh in the package namespace
  • Re-export UnknownProgramError from cuprum.sh and expose sh submodule publicly
  • Update all to include new types and facade module
cuprum/__init__.py
Extend behaviour tests to cover sh.make builder scenarios and SafeCmd metadata/argv contract
  • Add SafeCmd type-only import and new BDD step fixtures using Program and SafeCmd
  • Add given/when/then steps to build a SafeCmd via sh.make and assert argv_with_program contents and project metadata exposure
  • Add new catalogue.feature scenario describing safe command builder construction and expectations
tests/behaviour/test_catalogue_behaviour.py
tests/features/catalogue.feature
Add focused unit tests for SafeCmd construction, argv handling, and catalogue integration
  • Verify sh.make rejects unknown programs with UnknownProgramError
  • Assert SafeCmd instances contain correct program, argv, argv_with_program, and project metadata from the default catalogue
  • Test keyword argument serialization to flags, positional argument stringification (including Path), and custom catalogue metadata propagation
cuprum/unittests/test_sh.py
Update documentation and roadmap to describe the typed command core and mark feature completion
  • Extend user guide with section on typed command core, sh.make usage, argv rules, and project-specific builder examples
  • Add design doc implementation notes describing validation timing, argv normalization, None handling, and metadata exposure
  • Mark sh.make roadmap item as completed
docs/users-guide.md
docs/cuprum-design.md
docs/roadmap.md

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@coderabbitai

coderabbitai Bot commented Dec 2, 2025

Copy link
Copy Markdown

Warning

Rate limit exceeded

@leynos has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 12 minutes and 24 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between a2e89ce and 1814156.

📒 Files selected for processing (1)
  • docs/cuprum-design.md (4 hunks)

Note

Other AI code review bot(s) detected

CodeRabbit 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

Release Notes

  • New Features

    • Introduced typed command construction via new SafeCmd and SafeCmdBuilder, enabling safer command building with argument validation.
    • Added sh.make function to construct commands with automatic argument serialisation and error handling for unknown programmes.
  • Documentation

    • Added "Typed command core" section to user guide with usage examples and error handling details.
    • Updated design documentation with implementation notes and entity relationship diagrams.

✏️ Tip: You can customize this high-level summary in your review settings.

Walkthrough

Add a frozen SafeCmd dataclass and a make() builder factory in a new cuprum.sh module; re‑export SafeCmd, SafeCmdBuilder and the sh submodule at package level; add unit and behaviour tests and documentation for the typed command core. (≤50 words)

Changes

Cohort / File(s) Change Summary
New typed command module
cuprum/sh.py
Add SafeCmd frozen dataclass (program, argv, project) with argv_with_program; add SafeCmdBuilder type alias; implement _stringify_arg, _serialize_kwargs, _coerce_argv; add make(program, *, catalogue=DEFAULT_CATALOGUE) returning a builder; re‑export UnknownProgramError; set __all__.
Package-level exports
cuprum/__init__.py
Re‑export SafeCmd, SafeCmdBuilder and the sh submodule; update __all__ to include the new entries.
Unit tests
cuprum/unittests/test_sh.py
Add tests for unknown-program rejection, SafeCmd construction and metadata, kwarg serialisation and underscore→hyphen normalisation, positional arg stringification, None rejection, and custom catalogue injection.
Behaviour tests & feature
tests/behaviour/test_catalogue_behaviour.py, tests/features/catalogue.feature
Add scenario "Safe command builder constructs typed argv", fixtures and step implementations to build SafeCmd via sh.make and assert argv composition and project metadata exposure.
Documentation
docs/cuprum-design.md, docs/roadmap.md, docs/users-guide.md
Add "Typed command core" user guide; expand design doc with implementation notes and Mermaid diagrams; mark roadmap item complete; update examples and type‑hint formatting.

Sequence Diagram

sequenceDiagram
    participant Client
    participant sh as "cuprum.sh"
    participant Catalogue as "ProgrammeCatalogue"
    participant SafeCmdObj as "SafeCmd"

    Client->>sh: make(program_name)
    sh->>Catalogue: lookup(program_name)
    alt programme found
        Catalogue-->>sh: ProgrammeEntry
        sh-->>Client: builder(callable)
        Client->>builder: invoke(*args, **kwargs)
        builder->>builder: _coerce_argv(*args, **kwargs)
        builder->>builder: _stringify_arg(each)
        builder->>builder: _serialize_kwargs(**kwargs)
        builder->>SafeCmdObj: construct SafeCmd(program, argv, project)
        SafeCmdObj-->>Client: SafeCmd instance
    else programme not found
        Catalogue-->>sh: not found
        sh-->>Client: raise UnknownProgramError
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Inspect cuprum/sh.py argument‑handling functions (_stringify_arg, _serialize_kwargs, _coerce_argv) for edge cases and error messages.
  • Verify SafeCmdBuilder type alias matches the callable signature used in tests.
  • Confirm package re‑exports in cuprum/__init__.py do not create import cycles.
  • Review new tests (unittests and behaviour) for realistic fixtures and assertions.

Poem

A shell-builder hums with careful art,
Flags swap underscores and never part,
Args turn to strings and march in line,
Projects ride with metadata fine,
SafeCmd returns — the pipeline’s heart.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarises the main addition: a typed, safe command construction core with the SafeCmd dataclass and sh.make factory that validates programmes against a catalogue.
Description check ✅ Passed The description is comprehensive and directly related to the changeset, detailing the new typed command core, SafeCmd dataclass, sh.make factory, argument handling, public API exposure, tests, and documentation updates.
Docstring Coverage ✅ Passed Docstring coverage is 96.30% which is sufficient. The required threshold is 80.00%.

Comment @coderabbitai help to get the list of available commands and usage tips.

@leynos
leynos marked this pull request as ready for review December 2, 2025 17:49
sourcery-ai[bot]

This comment was marked as resolved.

- Refined SafeCmd typing to remove Out_co generic.
- Added support for Path in argument types.
- Normalized keyword argument names from underscores to hyphens.
- Reject None values as invalid positional or keyword args.
- Updated tests to cover these new behaviors.
- Improved docs with detailed design diagrams and clarifications.

Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
@leynos

leynos commented Dec 2, 2025

Copy link
Copy Markdown
Owner Author

@coderabbitai Has this now been resolved in the latest commit?

Use codegraph analysis to determine your answer.


suggestion (testing): Cover underscore-to-hyphen conversion for keyword flags

This test only covers simple kwarg → --k=v conversion and never passes a name containing an underscore. Please add or extend a test to call builder("hello", user_id=42) and assert that the flag is --user-id=42. That way the normalisation logic is exercised and the expected CLI flag style is explicitly captured in tests.

@leynos

leynos commented Dec 2, 2025

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Dec 2, 2025

Copy link
Copy Markdown
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented Dec 2, 2025

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Read the test file to examine test coverage
cat cuprum/unittests/test_sh.py

Length of output: 3682


🏁 Script executed:

#!/bin/bash
# Search for tests involving underscores in keyword arguments or hyphenation
rg -n -C5 'user_id|underscore|hyphen' cuprum/unittests/test_sh.py

Length of output: 655


🏁 Script executed:

#!/bin/bash
# Search for any keyword argument tests with underscores in flag assertions
rg -n -C3 '--.*-.*=' cuprum/unittests/test_sh.py

Length of output: 409


@leynos Yes, this has been resolved in the latest commit. ✅

The test file now includes test_keyword_arguments_normalise_underscores() (lines 59-65) which explicitly covers the underscore-to-hyphen conversion:

def test_keyword_arguments_normalise_underscores() -> None:
    """Kwarg names are normalised from underscores to hyphens."""
    builder = sh.make(ECHO)

    cmd = builder("hello", user_id=42)

    assert cmd.argv[-1] == "--user-id=42", "Underscores should become hyphens"

This test matches exactly what you requested in your original suggestion—it passes a keyword argument with an underscore (user_id=42) and asserts that it converts to the hyphenated flag format (--user-id=42). The normalisation logic is now explicitly exercised and captured in the test suite.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d7c5f1d and 1d8f9a5.

📒 Files selected for processing (8)
  • cuprum/__init__.py (2 hunks)
  • cuprum/sh.py (1 hunks)
  • cuprum/unittests/test_sh.py (1 hunks)
  • docs/cuprum-design.md (8 hunks)
  • docs/roadmap.md (1 hunks)
  • docs/users-guide.md (1 hunks)
  • tests/behaviour/test_catalogue_behaviour.py (5 hunks)
  • tests/features/catalogue.feature (1 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
docs/**/*.md

📄 CodeRabbit inference engine (AGENTS.md)

docs/**/*.md: Use markdown files within the docs/ directory as a knowledge base and source of truth for project requirements, dependency choices, and architectural decisions.
When new decisions are made, requirements change, libraries are added/removed, or architectural patterns evolve, proactively update the relevant file(s) in the docs/ directory to reflect the latest state.
All documentation must adhere to the documentation style guide at docs/documentation-style-guide.md.
Record any design decisions made in the relevant design document.

Files:

  • docs/users-guide.md
  • docs/cuprum-design.md
  • docs/roadmap.md
docs/users-guide.md

📄 CodeRabbit inference engine (AGENTS.md)

docs/users-guide.md: Ensure new functionality is clearly documented in the docs/users-guide.md file.
Ensure revised functionality is clearly documented in the docs/users-guide.md file.

docs/users-guide.md: Document the expected builder pattern in docs/users-guide.md
Provide a scaffold and guidance for project-specific builders, including a template module and checklist in docs/users-guide.md
Document policy switches and recommended defaults in docs/users-guide.md and add release notes describing the migration path for existing users

Files:

  • docs/users-guide.md
**/*.md

📄 CodeRabbit inference engine (AGENTS.md)

**/*.md: For Markdown files (.md only), ensure linting passes by running make markdownlint.
For Markdown files, validate Mermaid diagrams by running make nixie.
Markdown paragraphs and bullet points must be wrapped at 80 columns.
Code blocks in Markdown must be wrapped at 120 columns.
Markdown tables and headings must not be wrapped.
Use dashes (-) for list bullets in Markdown.
Use GitHub-flavoured Markdown footnotes ([^1]) for references and footnotes.
Validate Markdown files using make markdownlint.
Run make fmt after any documentation changes to format all Markdown files and fix table markup.
Validate Mermaid diagrams in Markdown files by running make nixie.

Files:

  • docs/users-guide.md
  • docs/cuprum-design.md
  • docs/roadmap.md

⚙️ CodeRabbit configuration file

**/*.md: * Avoid 2nd person or 1st person pronouns ("I", "you", "we")

  • Use en-GB-oxendict (-ize / -yse / -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:

  • docs/users-guide.md
  • docs/cuprum-design.md
  • docs/roadmap.md
docs/**/*.{md,mdx,rst,txt}

📄 CodeRabbit inference engine (docs/documentation-style-guide.md)

docs/**/*.{md,mdx,rst,txt}: Use British English based on Oxford English Dictionary (en-GB-oxendict) conventions: use -ize suffixes (realize, organization), -lyse suffixes (analyse, paralyse, catalyse), -our suffixes (colour, behaviour, neighbour), -re suffixes (calibre, centre, fibre), double 'l' (cancelled, counsellor, cruellest), maintain 'e' (likeable, liveable, rateable), -ogue suffixes (analogue, catalogue)
The word 'outwith' is acceptable in documentation
Use the Oxford comma in documentation: 'ships, planes, and hovercraft' where it aids comprehension
Treat company names as collective nouns in documentation, for example 'Lille Industries are expanding'
Write headings in sentence case in documentation
Use Markdown headings (#, ##, ###, and so on) in order without skipping levels
Always provide a language identifier for fenced code blocks in documentation; use 'plaintext' for non-code text
Use - as the first level bullet and renumber lists when items change in documentation
Prefer inline links using [text](url) or angle brackets around the URL in documentation
Ensure blank lines before and after bulleted lists and fenced blocks in documentation
Ensure tables have a delimiter line below the header row in documentation
Expand any uncommon acronym on first use in documentation, for example 'Continuous Integration (CI)'
Wrap paragraphs at 80 columns in documentation
Wrap code at 120 columns in documentation
Do not wrap tables in documentation
Use footnotes referenced with [^label] in documentation
Include Mermaid diagrams in documentation where they add clarity
When embedding figures in documentation, use ![alt text](path/to/image) and provide brief alt text describing the content
Add a short description before each Mermaid diagram in documentation so screen readers can understand it

Files:

  • docs/users-guide.md
  • docs/cuprum-design.md
  • docs/roadmap.md
docs/**/*.{md,mdx,rst,txt,rs}

📄 CodeRabbit inference engine (docs/documentation-style-guide.md)

Keep US spelling when used in API contexts, for example 'color'

Files:

  • docs/users-guide.md
  • docs/cuprum-design.md
  • docs/roadmap.md
docs/**/*.{md,mdx}

📄 CodeRabbit inference engine (docs/documentation-style-guide.md)

Follow markdownlint recommendations for Markdown formatting

Files:

  • docs/users-guide.md
  • docs/cuprum-design.md
  • docs/roadmap.md
**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

**/*.py: For Python files, ensure testing passes all relevant unit and behavioral tests by running make test.
For Python files, ensure linting passes by running make lint.
For Python files, ensure formatting adheres to standards by running make check-fmt and applying make fmt if needed.
For Python files, ensure type checking passes by running make typecheck.
For Python development, refer to Python-specific guidelines in the .rules/ directory, including Python Code Style Guidelines, Context Managers, Exceptions and Logging, Generators, Project Configuration, Return Patterns, and Typing.

**/*.py: Use snake_case for Python filenames (e.g., http_client.py, task_queue.py)
Use PascalCase for class names
Use snake_case for variable and function names
Use UPPER_SNAKE_CASE for module-level constants
Prefix private/internal functions and variables with a single underscore (_)
Enable full static type coverage using Pyright and maintain typing throughout the codebase
Use TypedDict or Dataclass for structured data, preferring @DataClass(slots=True) for internal-only usage
Avoid using Any type; use Unknown, generics, or cast() with documentation instead
Provide explicit return type annotations (e.g., -> None, -> str) for all public functions and class methods
Enforce strict mode in Pyright and treat all Pyright warnings as CI errors; use # pyright: ignore sparingly with explanation
Avoid side effects at import time; modules should not modify global state or perform actions on import
Use .env or settings modules for environment-specific configuration; never hardcode secrets
Use Ruff for formatting; let Ruff handle whitespace and formatting entirely
Use NumPy-format docstrings for public functions, classes, and modules
Use inline comments to explain tricky or non-obvious code logic and decisions

**/*.py: Use context managers (with contextlib.contextmanager or class-based __enter__/__exit__) to encapsulate setup and teardown logic for resource management (f...

Files:

  • cuprum/unittests/test_sh.py
  • cuprum/sh.py
  • tests/behaviour/test_catalogue_behaviour.py
  • cuprum/__init__.py

⚙️ CodeRabbit configuration file

**/*.py: - Keep C90 / mccabe complexity ≤ 9

  • Follow single responsibility and CQRS (command/query segregation)
  • Prefer structural pattern matching to
  • Prefer structural pattern matching over isinstance() or imperative decomposition.
  • 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 / -yse / -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.
  • All path manipulation must be performed using pathlib for cross platform safety. Do not use string manipulation for extraction of path components or filename ele...

Files:

  • cuprum/unittests/test_sh.py
  • cuprum/sh.py
  • tests/behaviour/test_catalogue_behaviour.py
  • cuprum/__init__.py
**/unittests/test_*.py

📄 CodeRabbit inference engine (.rules/python-00.md)

Colocate unit tests with code using an unittests subdirectory with test_ prefix (e.g., user_auth/unittests/test_models.py)

Files:

  • cuprum/unittests/test_sh.py
**/test_*.py

📄 CodeRabbit inference engine (.rules/python-00.md)

**/test_*.py: Use pytest idioms: prefer fixtures over setup/teardown methods, parametrize broadly, avoid unnecessary mocks
Group related tests using class with method names prefixed by test_
Write tests from a user's perspective; test public behavior rather than internals
Avoid mocking too much; prefer test doubles only for external services or non-deterministic behaviors

Files:

  • cuprum/unittests/test_sh.py
  • tests/behaviour/test_catalogue_behaviour.py
**/*test*.py

📄 CodeRabbit inference engine (.rules/python-exception-design-raising-handling-and-logging.md)

Use specific exception types and message constraints with pytest.raises(SpecificError, match=r"pattern") in tests; avoid overly broad exception assertions (B017)

Files:

  • cuprum/unittests/test_sh.py
  • tests/behaviour/test_catalogue_behaviour.py
🧬 Code graph analysis (4)
cuprum/unittests/test_sh.py (2)
cuprum/catalogue.py (3)
  • ProgramCatalogue (56-119)
  • ProjectSettings (30-40)
  • UnknownProgramError (25-26)
cuprum/sh.py (4)
  • make (76-92)
  • builder (88-90)
  • SafeCmd (63-73)
  • argv_with_program (71-73)
cuprum/sh.py (1)
cuprum/catalogue.py (4)
  • ProgramCatalogue (56-119)
  • ProjectSettings (30-40)
  • UnknownProgramError (25-26)
  • lookup (76-83)
tests/behaviour/test_catalogue_behaviour.py (2)
cuprum/catalogue.py (4)
  • ProgramCatalogue (56-119)
  • ProgramEntry (44-53)
  • ProjectSettings (30-40)
  • UnknownProgramError (25-26)
cuprum/sh.py (4)
  • SafeCmd (63-73)
  • builder (88-90)
  • make (76-92)
  • argv_with_program (71-73)
cuprum/__init__.py (1)
cuprum/sh.py (1)
  • SafeCmd (63-73)
🪛 LanguageTool
docs/users-guide.md

[uncategorized] ~46-~46: Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...nd carry project metadata alongside argv so downstream services can apply noise rul...

(COMMA_COMPOUND_SENTENCE_2)


[style] ~68-~68: Would you like to use the Oxford spelling “centralize”? The spelling ‘centralise’ is also correct.
Context: ...s Wrap sh.make in project modules to centralise validation and expose a clear API for c...

(OXFORD_SPELLING_Z_NOT_S)

docs/cuprum-design.md

[style] ~459-~459: Would you like to use the Oxford spelling “serialized”? The spelling ‘serialised’ is also correct.
Context: ...d with str(). Keyword arguments are serialised as --flag=value, replacing underscore...

(OXFORD_SPELLING_Z_NOT_S)


[style] ~467-~467: Would you like to use the Oxford spelling “summarizes”? The spelling ‘summarises’ is also correct.
Context: ... extra lookup. The following diagram summarises the relationships in the typed command ...

(OXFORD_SPELLING_Z_NOT_S)

🔍 Remote MCP

Let me try a different approach and search for the general cuprum project repository information:Based on the context provided in the PR summary, I have sufficient information to provide a concise review summary. The Deepwiki searches couldn't access the leynos/cuprum repository, but the AI summaries and PR objectives provide comprehensive details about the changes. Let me compile the relevant findings:

Summary of Key Review Context

Based on analysis of the provided PR documentation, here are the relevant facts for reviewing this pull request:

Core Component Integration

The PR introduces SafeCmd as a typed wrapper for command representations that integrates with existing Programme and ProgrammeCatalogue infrastructure. The implementation provides:

  • SafeCmd dataclass: Contains (program, argv, project) fields and an argv_with_program property that includes the program name in the argv
  • sh.make() factory: Validates programs exist in a catalogue before returning a builder, raising UnknownProgramError if not found
  • Argument handling: Supports positional argument stringification, keyword argument serialization to CLI flags (--flag=value), and underscore-to-hyphen conversion

Public API Expansion

The changes make SafeCmd, SafeCmdBuilder, and the sh submodule publicly available via cuprum/__init__.py, marking a new public surface area that requires stability considerations for future releases.

Test Coverage Status

Unit tests in cuprum/unittests/test_sh.py cover:

  • Unknown program rejection
  • Metadata population from catalogues
  • Keyword argument serialization
  • Non-string positional argument stringification
  • None value rejection with TypeError
  • Custom catalogue injection

Note from comments: There is an open testing suggestion to explicitly verify underscore-to-hyphen conversion with a test case like builder("hello", user_id=42) asserting --user-id=42. Current tests check simple kwarg conversion but may not explicitly exercise the normalization logic.

Documentation Updates

  • docs/cuprum-design.md: Added implementation notes with component relationship diagrams for the typed command core
  • docs/users-guide.md: New section documenting sh.make workflow, argument formatting, and error handling
  • docs/roadmap.md: sh.make task marked complete

Scope Clarification

SafeCmd is intentionally limited to command representation only; command execution functionality is out of scope for this PR.

⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Sourcery review
🔇 Additional comments (18)
docs/users-guide.md (1)

41-97: Documentation for the typed command core is clear and comprehensive.

The new section accurately documents the sh.make workflow, argument handling rules, and project-specific builder patterns. Examples align with the implementation in cuprum/sh.py.

docs/roadmap.md (1)

18-20: Roadmap update accurately reflects the delivered functionality.

The sh.make task is correctly marked complete, and the description aligns with the implemented SafeCmd builder and documentation additions.

docs/cuprum-design.md (1)

469-523: Mermaid diagram accurately captures the typed command core relationships.

The class diagram correctly illustrates the relationships between Program, ProgramCatalogue, ProgramEntry, SafeCmd, SafeCmdBuilder, and sh_module, aligning with the implementation in cuprum/sh.py and cuprum/catalogue.py.

tests/features/catalogue.feature (1)

19-23: Feature scenario provides good behavioural coverage for the SafeCmd builder.

The scenario exercises the sh.make facade, verifying argv construction and metadata exposure, aligning with the implementation in cuprum/sh.py.

cuprum/__init__.py (1)

30-32: Package exports correctly expose the new typed command core.

The imports and all additions properly expose SafeCmd, SafeCmdBuilder, and the sh submodule, enabling both direct imports (from cuprum import SafeCmd) and facade access (cuprum.sh.make()).

cuprum/unittests/test_sh.py (4)

59-65: Underscore-to-hyphen normalisation test addresses the past review concern.

This test explicitly exercises the flag name normalisation logic, confirming that user_id=42 becomes --user-id=42.


81-98: None rejection tests correctly lock in the expected behaviour.

Both positional and keyword argument None rejection are tested, ensuring the TypeError with "None" in the message is raised as specified in the documentation.


101-118: Custom catalogue injection test ensures metadata flows correctly.

The test verifies that a custom ProgramCatalogue's metadata is surfaced in the resulting SafeCmd, which is essential for downstream services.


68-78: Ensure Path objects are stringified consistently across platforms in _stringify_arg.

The test expects working_dir.as_posix() in argv, but if the implementation uses str(working_dir), this will fail on Windows because str(Path) returns backslash-separated paths while .as_posix() always returns forward slashes. Either update the implementation to explicitly call .as_posix() for Path objects, or align the test assertion to use str(working_dir) if that matches the actual implementation.

tests/behaviour/test_catalogue_behaviour.py (3)

102-108: Fixture creates Program without validating catalogue presence.

The given_curated_program fixture creates a Program directly without verifying it exists in DEFAULT_CATALOGUE. The validation happens in sh.make() during the When step, which is appropriate, but the Given step's docstring ("curated Program value for sh.make scenarios") could be clearer that catalogue validation is deferred.


129-140: When step correctly constructs SafeCmd via sh.make facade.

The step builds a SafeCmd using the provided arguments, exercising the builder pattern as intended.


188-209: Then steps provide appropriate assertions for argv and metadata.

The assertions verify both the argv_with_program structure and the exposure of project metadata (noise_rules, documentation_locations), ensuring downstream services can access the required information.

cuprum/sh.py (6)

1-7: Module docstring clearly establishes scope and intent.

The docstring effectively communicates the module's focus on typed command construction whilst explicitly excluding execution concerns. This boundary is essential for maintaining separation of concerns.


9-24: Imports follow best practices.

The use of deferred annotations, standard aliases (cabc), and the TYPE_CHECKING guard for Program demonstrates adherence to modern Python typing patterns and avoids circular import issues.


26-27: Type aliases use modern PEP 695 syntax correctly.

The explicit exclusion of None from _ArgValue paired with the runtime check in _stringify_arg provides both static and dynamic safety.


62-74: SafeCmd correctly implements frozen dataclass with proper type handling.

The use of str(self.program) in argv_with_program correctly addresses the past review concern, ensuring the returned tuple contains only str elements as per the annotation. The frozen=True, slots=True configuration follows guidelines for internal data structures.


95-95: Explicit __all__ correctly defines the public API surface.


30-59: Add explicit return type annotations to private helpers for Pyright strict mode compatibility.

Whilst the coding guidelines mandate return annotations only for public functions and class methods, Pyright in strict mode benefits from explicit annotations on all functions. Add -> str to _stringify_arg, -> tuple[str, ...] to _serialize_kwargs, and -> tuple[str, ...] to _coerce_argv.

Apply this diff to add return type annotations:

-def _stringify_arg(value: _ArgValue) -> str:
+def _stringify_arg(value: _ArgValue) -> str:
     """Convert values into argv-safe strings.
 
     ``None`` is disallowed because it is almost always a mistake in CLI argv
     construction. Callers should decide how to represent missing values (for
     example, omit the flag) before invoking ``sh.make``.
     """
     if value is None:
         msg = "None is not a valid argv element for sh.make"
         raise TypeError(msg)
     return str(value)


-def _serialize_kwargs(kwargs: dict[str, _ArgValue]) -> tuple[str, ...]:
+def _serialize_kwargs(kwargs: dict[str, _ArgValue]) -> tuple[str, ...]:
     """Serialise keyword arguments to CLI-style ``--flag=value`` entries."""
     flags: list[str] = []
     for key, value in kwargs.items():
         normalized_key = key.replace("_", "-")
         flags.append(f"--{normalized_key}={_stringify_arg(value)}")
     return tuple(flags)


-def _coerce_argv(
+def _coerce_argv(
     args: tuple[_ArgValue, ...],
     kwargs: dict[str, _ArgValue],
-) -> tuple[str, ...]:
+) -> tuple[str, ...]:
     """Convert positional and keyword arguments into a single argv tuple."""
     positional = tuple(_stringify_arg(arg) for arg in args)
     flags = _serialize_kwargs(kwargs)
     return positional + flags

Likely an incorrect or invalid review comment.

Comment thread cuprum/sh.py
Comment thread docs/cuprum-design.md Outdated
Comment thread docs/users-guide.md Outdated
Corrected spelling from British English to American English for consistency in the documentation:
- serialised -> serialized
- summarises -> summarizes
- centralise -> centralize

This improves clarity and aligns with the project's language style guidelines.

Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
@leynos

leynos commented Dec 3, 2025

Copy link
Copy Markdown
Owner Author

@coderabbitai have the following now been resolved?

  • The SafeCmd[Out_co] generic parameter is not currently used anywhere in the dataclass (all fields are concrete), which makes the type misleading; consider either wiring Out_co into the structure or dropping the generic until it is needed for execution typing.
  • The argv_with_program property is annotated as tuple[str, ...] but returns a tuple whose first element is a Program; it may be clearer to adjust the return type to reflect Program (or a Program | str union) depending on how Program is defined.
  • If you intend to encourage static typing for argument construction, you might consider tightening _ArgValue from object to a bounded union (e.g. str | int | Path | bool) or a SupportsStr protocol so that misuse is caught earlier by type checkers while still being stringified at runtime.

@coderabbitai

coderabbitai Bot commented Dec 3, 2025

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Read the sh.py file to check the current implementation
cat cuprum/sh.py

Length of output: 2985


@leynos Yes, all three issues have been resolved in the current implementation. ✅

  1. SafeCmd generic parameter: The SafeCmd[Out_co] generic has been dropped. The class is now a plain dataclass with no generic parameter (line 66):

    @dc.dataclass(frozen=True, slots=True)
    class SafeCmd:
  2. argv_with_program return type: The property now correctly returns tuple[str, ...] because it explicitly converts the Program to a string using str(self.program) (line 75):

    @property
    def argv_with_program(self) -> tuple[str, ...]:
        """Return argv prefixed with the program name."""
        return (str(self.program), *self.argv)
  3. _ArgValue type definition: The type has been tightened from object to a bounded union (line 25):

    type _ArgValue = str | int | float | bool | Path

    This provides better static type checking whilst still supporting stringification at runtime.

@leynos

leynos commented Dec 3, 2025

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Dec 3, 2025

Copy link
Copy Markdown
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1d8f9a5 and 240505a.

📒 Files selected for processing (2)
  • docs/cuprum-design.md (8 hunks)
  • docs/users-guide.md (1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
docs/**/*.md

📄 CodeRabbit inference engine (AGENTS.md)

docs/**/*.md: Use markdown files within the docs/ directory as a knowledge base and source of truth for project requirements, dependency choices, and architectural decisions.
When new decisions are made, requirements change, libraries are added/removed, or architectural patterns evolve, proactively update the relevant file(s) in the docs/ directory to reflect the latest state.
All documentation must adhere to the documentation style guide at docs/documentation-style-guide.md.
Record any design decisions made in the relevant design document.

Files:

  • docs/users-guide.md
  • docs/cuprum-design.md
docs/users-guide.md

📄 CodeRabbit inference engine (AGENTS.md)

docs/users-guide.md: Ensure new functionality is clearly documented in the docs/users-guide.md file.
Ensure revised functionality is clearly documented in the docs/users-guide.md file.

docs/users-guide.md: Document the expected builder pattern in docs/users-guide.md
Provide a scaffold and guidance for project-specific builders, including a template module and checklist in docs/users-guide.md
Document policy switches and recommended defaults in docs/users-guide.md and add release notes describing the migration path for existing users

Files:

  • docs/users-guide.md
**/*.md

📄 CodeRabbit inference engine (AGENTS.md)

**/*.md: For Markdown files (.md only), ensure linting passes by running make markdownlint.
For Markdown files, validate Mermaid diagrams by running make nixie.
Markdown paragraphs and bullet points must be wrapped at 80 columns.
Code blocks in Markdown must be wrapped at 120 columns.
Markdown tables and headings must not be wrapped.
Use dashes (-) for list bullets in Markdown.
Use GitHub-flavoured Markdown footnotes ([^1]) for references and footnotes.
Validate Markdown files using make markdownlint.
Run make fmt after any documentation changes to format all Markdown files and fix table markup.
Validate Mermaid diagrams in Markdown files by running make nixie.

Files:

  • docs/users-guide.md
  • docs/cuprum-design.md

⚙️ CodeRabbit configuration file

**/*.md: * Avoid 2nd person or 1st person pronouns ("I", "you", "we")

  • Use en-GB-oxendict (-ize / -yse / -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:

  • docs/users-guide.md
  • docs/cuprum-design.md
docs/**/*.{md,mdx,rst,txt}

📄 CodeRabbit inference engine (docs/documentation-style-guide.md)

docs/**/*.{md,mdx,rst,txt}: Use British English based on Oxford English Dictionary (en-GB-oxendict) conventions: use -ize suffixes (realize, organization), -lyse suffixes (analyse, paralyse, catalyse), -our suffixes (colour, behaviour, neighbour), -re suffixes (calibre, centre, fibre), double 'l' (cancelled, counsellor, cruellest), maintain 'e' (likeable, liveable, rateable), -ogue suffixes (analogue, catalogue)
The word 'outwith' is acceptable in documentation
Use the Oxford comma in documentation: 'ships, planes, and hovercraft' where it aids comprehension
Treat company names as collective nouns in documentation, for example 'Lille Industries are expanding'
Write headings in sentence case in documentation
Use Markdown headings (#, ##, ###, and so on) in order without skipping levels
Always provide a language identifier for fenced code blocks in documentation; use 'plaintext' for non-code text
Use - as the first level bullet and renumber lists when items change in documentation
Prefer inline links using [text](url) or angle brackets around the URL in documentation
Ensure blank lines before and after bulleted lists and fenced blocks in documentation
Ensure tables have a delimiter line below the header row in documentation
Expand any uncommon acronym on first use in documentation, for example 'Continuous Integration (CI)'
Wrap paragraphs at 80 columns in documentation
Wrap code at 120 columns in documentation
Do not wrap tables in documentation
Use footnotes referenced with [^label] in documentation
Include Mermaid diagrams in documentation where they add clarity
When embedding figures in documentation, use ![alt text](path/to/image) and provide brief alt text describing the content
Add a short description before each Mermaid diagram in documentation so screen readers can understand it

Files:

  • docs/users-guide.md
  • docs/cuprum-design.md
docs/**/*.{md,mdx,rst,txt,rs}

📄 CodeRabbit inference engine (docs/documentation-style-guide.md)

Keep US spelling when used in API contexts, for example 'color'

Files:

  • docs/users-guide.md
  • docs/cuprum-design.md
docs/**/*.{md,mdx}

📄 CodeRabbit inference engine (docs/documentation-style-guide.md)

Follow markdownlint recommendations for Markdown formatting

Files:

  • docs/users-guide.md
  • docs/cuprum-design.md
🪛 LanguageTool
docs/users-guide.md

[uncategorized] ~46-~46: Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...nd carry project metadata alongside argv so downstream services can apply noise rul...

(COMMA_COMPOUND_SENTENCE_2)

⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Sourcery review
🔇 Additional comments (2)
docs/users-guide.md (1)

87-87: Verify generic parameter on SafeCmd type annotation.

The documentation shows sh.SafeCmd[str] with a generic parameter, but the PR objectives state that the SafeCmd generic was removed and is now a plain dataclass. Clarify whether this example should use sh.SafeCmd without parameterisation, or document the generic's purpose if it is still present.

docs/cuprum-design.md (1)

467-523: Validate Mermaid diagram syntax.

Run make nixie to confirm the Mermaid diagram (lines 469–523) validates without errors. Ensure all class names, relationships, and generic parameter syntax are correct.

Comment thread docs/cuprum-design.md Outdated
Comment thread docs/cuprum-design.md
Comment thread docs/cuprum-design.md Outdated
Comment thread docs/users-guide.md Outdated
- Added heading markup for implementation notes in cuprum-design.md
- Fixed multiline code formatting and imports for clarity
- Improved code snippet formatting for async commands
- Corrected tuple ellipsis character
- Adjusted line wrapping and punctuation for better readability
- Minor text tweaks for users-guide.md formatting consistency

Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 240505a and 05e042b.

📒 Files selected for processing (2)
  • docs/cuprum-design.md (2 hunks)
  • docs/users-guide.md (1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
docs/**/*.md

📄 CodeRabbit inference engine (AGENTS.md)

docs/**/*.md: Use markdown files within the docs/ directory as a knowledge base and source of truth for project requirements, dependency choices, and architectural decisions.
When new decisions are made, requirements change, libraries are added/removed, or architectural patterns evolve, proactively update the relevant file(s) in the docs/ directory to reflect the latest state.
All documentation must adhere to the documentation style guide at docs/documentation-style-guide.md.
Record any design decisions made in the relevant design document.

Files:

  • docs/users-guide.md
  • docs/cuprum-design.md
docs/users-guide.md

📄 CodeRabbit inference engine (AGENTS.md)

docs/users-guide.md: Ensure new functionality is clearly documented in the docs/users-guide.md file.
Ensure revised functionality is clearly documented in the docs/users-guide.md file.

docs/users-guide.md: Document the expected builder pattern in docs/users-guide.md
Provide a scaffold and guidance for project-specific builders, including a template module and checklist in docs/users-guide.md
Document policy switches and recommended defaults in docs/users-guide.md and add release notes describing the migration path for existing users

Files:

  • docs/users-guide.md
**/*.md

📄 CodeRabbit inference engine (AGENTS.md)

**/*.md: For Markdown files (.md only), ensure linting passes by running make markdownlint.
For Markdown files, validate Mermaid diagrams by running make nixie.
Markdown paragraphs and bullet points must be wrapped at 80 columns.
Code blocks in Markdown must be wrapped at 120 columns.
Markdown tables and headings must not be wrapped.
Use dashes (-) for list bullets in Markdown.
Use GitHub-flavoured Markdown footnotes ([^1]) for references and footnotes.
Validate Markdown files using make markdownlint.
Run make fmt after any documentation changes to format all Markdown files and fix table markup.
Validate Mermaid diagrams in Markdown files by running make nixie.

Files:

  • docs/users-guide.md
  • docs/cuprum-design.md

⚙️ CodeRabbit configuration file

**/*.md: * Avoid 2nd person or 1st person pronouns ("I", "you", "we")

  • Use en-GB-oxendict (-ize / -yse / -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:

  • docs/users-guide.md
  • docs/cuprum-design.md
docs/**/*.{md,mdx,rst,txt}

📄 CodeRabbit inference engine (docs/documentation-style-guide.md)

docs/**/*.{md,mdx,rst,txt}: Use British English based on Oxford English Dictionary (en-GB-oxendict) conventions: use -ize suffixes (realize, organization), -lyse suffixes (analyse, paralyse, catalyse), -our suffixes (colour, behaviour, neighbour), -re suffixes (calibre, centre, fibre), double 'l' (cancelled, counsellor, cruellest), maintain 'e' (likeable, liveable, rateable), -ogue suffixes (analogue, catalogue)
The word 'outwith' is acceptable in documentation
Use the Oxford comma in documentation: 'ships, planes, and hovercraft' where it aids comprehension
Treat company names as collective nouns in documentation, for example 'Lille Industries are expanding'
Write headings in sentence case in documentation
Use Markdown headings (#, ##, ###, and so on) in order without skipping levels
Always provide a language identifier for fenced code blocks in documentation; use 'plaintext' for non-code text
Use - as the first level bullet and renumber lists when items change in documentation
Prefer inline links using [text](url) or angle brackets around the URL in documentation
Ensure blank lines before and after bulleted lists and fenced blocks in documentation
Ensure tables have a delimiter line below the header row in documentation
Expand any uncommon acronym on first use in documentation, for example 'Continuous Integration (CI)'
Wrap paragraphs at 80 columns in documentation
Wrap code at 120 columns in documentation
Do not wrap tables in documentation
Use footnotes referenced with [^label] in documentation
Include Mermaid diagrams in documentation where they add clarity
When embedding figures in documentation, use ![alt text](path/to/image) and provide brief alt text describing the content
Add a short description before each Mermaid diagram in documentation so screen readers can understand it

Files:

  • docs/users-guide.md
  • docs/cuprum-design.md
docs/**/*.{md,mdx,rst,txt,rs}

📄 CodeRabbit inference engine (docs/documentation-style-guide.md)

Keep US spelling when used in API contexts, for example 'color'

Files:

  • docs/users-guide.md
  • docs/cuprum-design.md
docs/**/*.{md,mdx}

📄 CodeRabbit inference engine (docs/documentation-style-guide.md)

Follow markdownlint recommendations for Markdown formatting

Files:

  • docs/users-guide.md
  • docs/cuprum-design.md
⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Sourcery review
🔇 Additional comments (1)
docs/cuprum-design.md (1)

512-522: This review comment is incorrect. The SafeCmd class definition still uses Generic[Out] in the source code, so the Mermaid diagram representation using SafeCmd~Out_co~ (covariant generic) remains appropriate and accurate. No changes to these diagram relationships are needed.

Likely an incorrect or invalid review comment.

Comment thread docs/cuprum-design.md Outdated
Comment thread docs/users-guide.md Outdated
…usage example

Updated the class diagram in cuprum-design.md to replace outdated SafeCmd~Out_co~ and SafeCmd_str references with SafeCmd. Adjusted SafeCmdBuilder connections accordingly. Also corrected import and type annotation in users-guide.md to reflect the updated SafeCmd usage.

Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
@leynos leynos changed the title Add typed SafeCmd core and sh.make with safe argv handling Implement typed command core with safety Dec 3, 2025

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 05e042b and 421cff2.

📒 Files selected for processing (2)
  • docs/cuprum-design.md (2 hunks)
  • docs/users-guide.md (1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
docs/**/*.md

📄 CodeRabbit inference engine (AGENTS.md)

docs/**/*.md: Use markdown files within the docs/ directory as a knowledge base and source of truth for project requirements, dependency choices, and architectural decisions.
When new decisions are made, requirements change, libraries are added/removed, or architectural patterns evolve, proactively update the relevant file(s) in the docs/ directory to reflect the latest state.
All documentation must adhere to the documentation style guide at docs/documentation-style-guide.md.
Record any design decisions made in the relevant design document.

Files:

  • docs/users-guide.md
  • docs/cuprum-design.md
docs/users-guide.md

📄 CodeRabbit inference engine (AGENTS.md)

docs/users-guide.md: Ensure new functionality is clearly documented in the docs/users-guide.md file.
Ensure revised functionality is clearly documented in the docs/users-guide.md file.

docs/users-guide.md: Document the expected builder pattern in docs/users-guide.md
Provide a scaffold and guidance for project-specific builders, including a template module and checklist in docs/users-guide.md
Document policy switches and recommended defaults in docs/users-guide.md and add release notes describing the migration path for existing users

Files:

  • docs/users-guide.md
**/*.md

📄 CodeRabbit inference engine (AGENTS.md)

**/*.md: For Markdown files (.md only), ensure linting passes by running make markdownlint.
For Markdown files, validate Mermaid diagrams by running make nixie.
Markdown paragraphs and bullet points must be wrapped at 80 columns.
Code blocks in Markdown must be wrapped at 120 columns.
Markdown tables and headings must not be wrapped.
Use dashes (-) for list bullets in Markdown.
Use GitHub-flavoured Markdown footnotes ([^1]) for references and footnotes.
Validate Markdown files using make markdownlint.
Run make fmt after any documentation changes to format all Markdown files and fix table markup.
Validate Mermaid diagrams in Markdown files by running make nixie.

Files:

  • docs/users-guide.md
  • docs/cuprum-design.md

⚙️ CodeRabbit configuration file

**/*.md: * Avoid 2nd person or 1st person pronouns ("I", "you", "we")

  • Use en-GB-oxendict (-ize / -yse / -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:

  • docs/users-guide.md
  • docs/cuprum-design.md
docs/**/*.{md,mdx,rst,txt}

📄 CodeRabbit inference engine (docs/documentation-style-guide.md)

docs/**/*.{md,mdx,rst,txt}: Use British English based on Oxford English Dictionary (en-GB-oxendict) conventions: use -ize suffixes (realize, organization), -lyse suffixes (analyse, paralyse, catalyse), -our suffixes (colour, behaviour, neighbour), -re suffixes (calibre, centre, fibre), double 'l' (cancelled, counsellor, cruellest), maintain 'e' (likeable, liveable, rateable), -ogue suffixes (analogue, catalogue)
The word 'outwith' is acceptable in documentation
Use the Oxford comma in documentation: 'ships, planes, and hovercraft' where it aids comprehension
Treat company names as collective nouns in documentation, for example 'Lille Industries are expanding'
Write headings in sentence case in documentation
Use Markdown headings (#, ##, ###, and so on) in order without skipping levels
Always provide a language identifier for fenced code blocks in documentation; use 'plaintext' for non-code text
Use - as the first level bullet and renumber lists when items change in documentation
Prefer inline links using [text](url) or angle brackets around the URL in documentation
Ensure blank lines before and after bulleted lists and fenced blocks in documentation
Ensure tables have a delimiter line below the header row in documentation
Expand any uncommon acronym on first use in documentation, for example 'Continuous Integration (CI)'
Wrap paragraphs at 80 columns in documentation
Wrap code at 120 columns in documentation
Do not wrap tables in documentation
Use footnotes referenced with [^label] in documentation
Include Mermaid diagrams in documentation where they add clarity
When embedding figures in documentation, use ![alt text](path/to/image) and provide brief alt text describing the content
Add a short description before each Mermaid diagram in documentation so screen readers can understand it

Files:

  • docs/users-guide.md
  • docs/cuprum-design.md
docs/**/*.{md,mdx,rst,txt,rs}

📄 CodeRabbit inference engine (docs/documentation-style-guide.md)

Keep US spelling when used in API contexts, for example 'color'

Files:

  • docs/users-guide.md
  • docs/cuprum-design.md
docs/**/*.{md,mdx}

📄 CodeRabbit inference engine (docs/documentation-style-guide.md)

Follow markdownlint recommendations for Markdown formatting

Files:

  • docs/users-guide.md
  • docs/cuprum-design.md
⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Sourcery review
🔇 Additional comments (1)
docs/users-guide.md (1)

42-97: ✓ Approved: Documentation is well-structured and addresses earlier feedback.

The new "Typed command core" section integrates cleanly, with correct imports, proper return type annotations (SafeCmd not sh.SafeCmd[str]), compliant column wrapping, and idiomatic code examples. All prior review comments (spelling, punctuation, type annotations) have been addressed.

Comment thread docs/cuprum-design.md
Corrected the return type annotation of __call__ method in SafeCmdBuilder from 'SafeCmd~str~' to 'SafeCmd'.
Also simplified the classDiagram references by replacing templated types like 'SafeCmd_str' and 'SafeCmd~Out_co~' with 'SafeCmd'. This improves documentation clarity and consistency.

Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 (1)
docs/cuprum-design.md (1)

251-252: Synchronise documentation with the plain SafeCmd dataclass implementation.

Sections 5.2 (line 251) and 6.1.2 (line 330) still describe SafeCmd as a generic type (SafeCmd[Out]), and the type sketch at line 342 shows class SafeCmd(Generic[Out]). However, the implementation notes (lines 463–465) and the Mermaid diagram (line 486) correctly reflect that SafeCmd is now a plain dataclass without generics.

Update sections 5.2, 6.1.2, and the type sketch at line 342 to remove all references to the generic parameter and present SafeCmd as a non-generic dataclass. Ensure the overall document presents a consistent story of SafeCmd's structure throughout.

Also applies to: 330-332, 342-342

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 421cff2 and a2e89ce.

📒 Files selected for processing (1)
  • docs/cuprum-design.md (2 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
docs/**/*.md

📄 CodeRabbit inference engine (AGENTS.md)

docs/**/*.md: Use markdown files within the docs/ directory as a knowledge base and source of truth for project requirements, dependency choices, and architectural decisions.
When new decisions are made, requirements change, libraries are added/removed, or architectural patterns evolve, proactively update the relevant file(s) in the docs/ directory to reflect the latest state.
All documentation must adhere to the documentation style guide at docs/documentation-style-guide.md.
Record any design decisions made in the relevant design document.

Files:

  • docs/cuprum-design.md
**/*.md

📄 CodeRabbit inference engine (AGENTS.md)

**/*.md: For Markdown files (.md only), ensure linting passes by running make markdownlint.
For Markdown files, validate Mermaid diagrams by running make nixie.
Markdown paragraphs and bullet points must be wrapped at 80 columns.
Code blocks in Markdown must be wrapped at 120 columns.
Markdown tables and headings must not be wrapped.
Use dashes (-) for list bullets in Markdown.
Use GitHub-flavoured Markdown footnotes ([^1]) for references and footnotes.
Validate Markdown files using make markdownlint.
Run make fmt after any documentation changes to format all Markdown files and fix table markup.
Validate Mermaid diagrams in Markdown files by running make nixie.

Files:

  • docs/cuprum-design.md

⚙️ CodeRabbit configuration file

**/*.md: * Avoid 2nd person or 1st person pronouns ("I", "you", "we")

  • Use en-GB-oxendict (-ize / -yse / -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:

  • docs/cuprum-design.md
docs/**/*.{md,mdx,rst,txt}

📄 CodeRabbit inference engine (docs/documentation-style-guide.md)

docs/**/*.{md,mdx,rst,txt}: Use British English based on Oxford English Dictionary (en-GB-oxendict) conventions: use -ize suffixes (realize, organization), -lyse suffixes (analyse, paralyse, catalyse), -our suffixes (colour, behaviour, neighbour), -re suffixes (calibre, centre, fibre), double 'l' (cancelled, counsellor, cruellest), maintain 'e' (likeable, liveable, rateable), -ogue suffixes (analogue, catalogue)
The word 'outwith' is acceptable in documentation
Use the Oxford comma in documentation: 'ships, planes, and hovercraft' where it aids comprehension
Treat company names as collective nouns in documentation, for example 'Lille Industries are expanding'
Write headings in sentence case in documentation
Use Markdown headings (#, ##, ###, and so on) in order without skipping levels
Always provide a language identifier for fenced code blocks in documentation; use 'plaintext' for non-code text
Use - as the first level bullet and renumber lists when items change in documentation
Prefer inline links using [text](url) or angle brackets around the URL in documentation
Ensure blank lines before and after bulleted lists and fenced blocks in documentation
Ensure tables have a delimiter line below the header row in documentation
Expand any uncommon acronym on first use in documentation, for example 'Continuous Integration (CI)'
Wrap paragraphs at 80 columns in documentation
Wrap code at 120 columns in documentation
Do not wrap tables in documentation
Use footnotes referenced with [^label] in documentation
Include Mermaid diagrams in documentation where they add clarity
When embedding figures in documentation, use ![alt text](path/to/image) and provide brief alt text describing the content
Add a short description before each Mermaid diagram in documentation so screen readers can understand it

Files:

  • docs/cuprum-design.md
docs/**/*.{md,mdx,rst,txt,rs}

📄 CodeRabbit inference engine (docs/documentation-style-guide.md)

Keep US spelling when used in API contexts, for example 'color'

Files:

  • docs/cuprum-design.md
docs/**/*.{md,mdx}

📄 CodeRabbit inference engine (docs/documentation-style-guide.md)

Follow markdownlint recommendations for Markdown formatting

Files:

  • docs/cuprum-design.md
⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Sourcery review
🔇 Additional comments (1)
docs/cuprum-design.md (1)

461-464: Fix line wrapping to comply with 80-column guideline.

Lines 461 and 464 exceed the 80-character limit:

  • Line 461: "- None is rejected as an argument value to catch accidental omissions early;" contains 81 characters.
  • Line 464: " noise rules and documentation links visible to downstream hooks without an" contains 82 characters.

Reflow these lines to fit within 80 columns:

- Lines 461–462: Shorten or split the `None` rejection clause so each line is ≤80 chars.
- Lines 463–465: Reflow the SafeCmd clause so line 464 no longer exceeds the limit.

Comment thread docs/cuprum-design.md Outdated
…ntation

- Removed the generic `[Out]` type parameter from `SafeCmd` references.
- Updated related type hints and docstrings to reflect this change.
- Improved formatting and fixed annotation style for `ExecEvent` dataclass.
- Clarified descriptions for SafeCmd and DynamicCmd in the documentation.

Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
@leynos
leynos merged commit a47762d into main Dec 4, 2025
4 checks passed
@leynos
leynos deleted the terragon/implement-typed-command-core-gq70bq branch December 4, 2025 02:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant