Introduce typed Program core and safety catalogue - #6
Merged
Conversation
![terragon-labs[bot]](https://github.githubassets.com/images/gravatars/gravatar-user-420.png){kind=avatar}
…adata Introduces a new `Program` NewType for typed executables and a `ProgramCatalogue` class that maintains a curated allowlist of allowed programs grouped by `ProjectSettings`. The catalogue includes functionality to lookup programs, enforce allowlist restrictions (by raising `UnknownProgramError` for unknown executables), and expose project metadata such as noise rules and documentation locations for downstream services. Adds unit and behavioral tests covering the catalogue usage and integration with pytest-bdd. Updates documentation accordingly to describe the new catalogue feature and usage guidance. Also updates dependencies to include `pytest-bdd` for behavioral tests. This implementation enhances safety by blocking unknown commands by default and improves code maintainability and clarity by centralizing executable definitions with associated metadata. Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
|
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 CodeRabbitRelease Notes
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughIntroduce a typed Program NewType and a ProgramCatalogue with ProjectSettings and ProgramEntry; add curated program constants and a default catalogue; export these symbols at package level; add unit and BDD tests and update documentation and dev dependencies. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Contributor
Reviewer's GuideImplements a typed Program abstraction and a curated ProgramCatalogue with a default allowlist that blocks unknown executables, exposes project metadata via a read-only API, wires the new catalogue into the public cuprum API, and adds tests, docs, and dependency updates to support the new safety model. Sequence diagram for ProgramCatalogue.lookup and safety checkssequenceDiagram
actor Developer
participant Catalogue as ProgramCatalogue
participant Helper as _coerce_program
participant Map as _program_to_project
participant Entry as ProgramEntry
participant Error as UnknownProgramError
Developer->>Catalogue: lookup(raw_program)
Catalogue->>Helper: _coerce_program(raw_program)
Helper-->>Catalogue: program_value
Catalogue->>Map: get(program_value)
Map-->>Catalogue: project_or_none
alt program not found
Catalogue->>Error: construct with message
Catalogue-->>Developer: raise UnknownProgramError
else program found
Catalogue->>Entry: construct(program_value, project)
Entry-->>Catalogue: entry
Catalogue-->>Developer: ProgramEntry
end
Sequence diagram for visible_settings metadata exposuresequenceDiagram
actor DownstreamService
participant Catalogue as ProgramCatalogue
DownstreamService->>Catalogue: visible_settings()
Catalogue-->>DownstreamService: MappingProxyType(dict(_projects))
DownstreamService->>DownstreamService: iterate projects
DownstreamService->>DownstreamService: read documentation_locations, noise_rules
Class diagram for typed Program core and ProgramCatalogueclassDiagram
class Program {
}
class UnknownProgramError {
<<exception>>
}
class ProjectSettings {
+str name
+tuple~Program~ programs
+tuple~str~ documentation_locations
+tuple~str~ noise_rules
+owns(program: Program): bool
}
class ProgramEntry {
+Program program
+ProjectSettings project
+project_name: str
}
class ProgramCatalogue {
-dict~str, ProjectSettings~ _projects
-dict~Program, ProjectSettings~ _program_to_project
-frozenset~Program~ _allowlist
+ProgramCatalogue(projects: Iterable~ProjectSettings~)
+allowlist: frozenset~Program~
+is_allowed(program: Program): bool
+lookup(program: Program | str): ProgramEntry
+project_for(program: Program | str): ProjectSettings
+visible_settings(): Mapping~str, ProjectSettings~
+_index_projects(projects: Iterable~ProjectSettings~) dict~str, ProjectSettings~
+_index_programs(projects: dict~str, ProjectSettings~) dict~Program, ProjectSettings~
}
class DEFAULTS {
<<module level constants>>
+str CORE_OPS_PROJECT
+str DOCUMENTATION_PROJECT
+Program ECHO
+Program LS
+Program DOC_TOOL
+tuple~ProjectSettings~ DEFAULT_PROJECTS
+ProgramCatalogue DEFAULT_CATALOGUE
}
ProgramCatalogue --> ProgramEntry : returns
ProgramCatalogue --> ProjectSettings : indexes
ProgramCatalogue --> UnknownProgramError : raises
ProgramEntry --> ProjectSettings : project
ProjectSettings --> Program : owns
DEFAULTS --> ProjectSettings : constructs
DEFAULTS --> ProgramCatalogue : constructs
DEFAULTS --> Program : constructs
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
leynos
marked this pull request as ready for review
December 2, 2025 01:16
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey there - I've reviewed your changes and found some issues that need to be addressed.
- In
visible_settings()you wrap a newdict(self._projects)inMappingProxyType, which is an unnecessary copy given that_projectsis only populated at construction time; consider returningMappingProxyType(self._projects)instead for a cheaper, still read‑only view. DOCUMENTATION_PROJECTis defined incatalogue.pybut not exported via__all__or the top‑levelcuprumpackage and is not referenced elsewhere in this diff; consider either exporting it consistently likeCORE_OPS_PROJECTor inlining/removing it if you don’t expect callers to depend on it.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `visible_settings()` you wrap a new `dict(self._projects)` in `MappingProxyType`, which is an unnecessary copy given that `_projects` is only populated at construction time; consider returning `MappingProxyType(self._projects)` instead for a cheaper, still read‑only view.
- `DOCUMENTATION_PROJECT` is defined in `catalogue.py` but not exported via `__all__` or the top‑level `cuprum` package and is not referenced elsewhere in this diff; consider either exporting it consistently like `CORE_OPS_PROJECT` or inlining/removing it if you don’t expect callers to depend on it.
## Individual Comments
### Comment 1
<location> `cuprum/catalogue.py:57-59` </location>
<code_context>
+ """Return the curated allowlist of programs."""
+ return self._allowlist
+
+ def is_allowed(self, program: Program) -> bool:
+ """Return True when the program is part of the default allowlist."""
+ return program in self._allowlist
+
+ def lookup(self, program: Program | str) -> ProgramEntry:
</code_context>
<issue_to_address>
**suggestion:** Consider accepting `Program | str` in `is_allowed` for consistency with `lookup`.
This asymmetry means callers who only need a boolean allowlist check must depend on the `Program` NewType or wrap strings themselves. Allowing `Program | str` here and delegating to `_coerce_program` would improve ergonomics and keep the public API consistent.
```suggestion
def is_allowed(self, program: Program | str) -> bool:
"""Return True when the program is part of the default allowlist."""
program_value = _coerce_program(program)
return program_value in self._allowlist
```
</issue_to_address>
### Comment 2
<location> `cuprum/unittests/test_catalogue.py:31` </location>
<code_context>
+ assert CORE_OPS_PROJECT in DEFAULT_CATALOGUE.visible_settings()
+
+
+def test_unknown_programs_are_blocked_by_default() -> None:
+ """Unknown executables are rejected to maintain safety by default."""
+ with pytest.raises(UnknownProgramError):
</code_context>
<issue_to_address>
**issue (testing):** Add unit tests for duplicate projects/programs to exercise catalogue safety guards.
The catalogue has explicit guards against duplicate project names and duplicate program ownership (`_index_projects` and `_index_programs` raise `ValueError`), but there are no tests covering them. Please add tests such as:
- Constructing `ProgramCatalogue` with two `ProjectSettings` sharing the same `name` raises `ValueError` with a clear message.
- Constructing `ProgramCatalogue` where two different projects list the same `Program` in `programs` raises `ValueError` and identifies the original owner.
This will help ensure the uniqueness constraints remain enforced across refactors.
</issue_to_address>
### Comment 3
<location> `cuprum/catalogue.py:129` </location>
<code_context>
+ ),
+)
+
+DEFAULT_CATALOGUE = ProgramCatalogue(projects=DEFAULT_PROJECTS)
+
+__all__ = [
</code_context>
<issue_to_address>
**issue (review_instructions):** Add unit and behavioural tests that exercise ProgramCatalogue, ProjectSettings, ProgramEntry, and UnknownProgramError.
Add unit tests under `cuprum/unittests/` that cover at least:
- successful lookups of curated programs (e.g. `ECHO`, `LS`, `DOC_TOOL`) and their owning `ProjectSettings`;
- `is_allowed`/`allowlist` behaviour;
- `visible_settings()` returning the expected metadata and being read‑only;
- duplicate project names and duplicate program ownership raising `ValueError`;
- `UnknownProgramError` when looking up a program not in the allowlist.
Add behavioural tests under `tests/behaviour` (and corresponding `.feature` coverage if you are using BDD) that demonstrate the externally visible behaviour of this catalogue: unknown executables being rejected by default, curated programs being accepted with the right metadata, and downstream components being able to consume `visible_settings()`.
Ensure the newly added empty test files are populated with concrete test cases so this feature is fully covered at both the unit and behavioural levels.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `**/*`
**Instructions:**
For any new feature or change to an existing feature, both behavioural *and* unit tests are required.
</details>
</issue_to_address>
### Comment 4
<location> `cuprum/program.py:7` </location>
<code_context>
+
+import typing as typ
+
+Program = typ.NewType("Program", str)
+
+__all__ = ["Program"]
</code_context>
<issue_to_address>
**issue (review_instructions):** Add tests covering the Program NewType usage and coercion behaviour.
Add unit tests that exercise `Program` as a typed wrapper around `str`, including interactions via `_coerce_program` and use as dictionary keys in the catalogue. Verify that equality, hashing, and membership checks behave as intended when mixing `Program` instances and plain strings (where supported) so regressions in the typed command core are caught.
Also extend the behavioural tests for the catalogue so that end‑to‑end scenarios assert that commands are passed around as `Program` values rather than raw strings, fulfilling the requirement for behavioural coverage of this new typed core.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `**/*`
**Instructions:**
For any new feature or change to an existing feature, both behavioural *and* unit tests are required.
</details>
</issue_to_address>
### Comment 5
<location> `cuprum/__init__.py:21` </location>
<code_context>
+
PACKAGE_NAME = "cuprum"
+__all__ = [
+ "CORE_OPS_PROJECT",
+ "DEFAULT_CATALOGUE",
</code_context>
<issue_to_address>
**issue (review_instructions):** Cover the newly exposed public API symbols with tests to guard their behaviour and availability.
Add unit tests that import the new symbols from `cuprum`’s top‑level package (`DEFAULT_CATALOGUE`, `DEFAULT_PROJECTS`, `CORE_OPS_PROJECT`, `DOC_TOOL`, `ECHO`, `LS`, `Program`, `ProgramCatalogue`, `ProgramEntry`, `ProjectSettings`, `UnknownProgramError`) and assert they are present and wired to the expected underlying objects.
Add behavioural tests that exercise the public surface via `import cuprum as c` (or similar) and use only this re‑exported API to perform catalogue lookups and allowlist checks. This will ensure that the newly added public surface is covered in both unit and behavioural suites, as required.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `**/*`
**Instructions:**
For any new feature or change to an existing feature, both behavioural *and* unit tests are required.
</details>
</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: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cuprum/__init__.py (1)
1-1: Expand the module docstring to meet documentation requirements.Per coding guidelines, module docstrings must explain purpose, utility, and usage with example calls if appropriate. The current docstring is insufficient.
-"""cuprum package.""" +""" +cuprum package. + +Provides a typed programme catalogue system for managing curated, allowlisted +executables. Re-exports core types and the default catalogue for convenience. + +Example +------- +>>> from cuprum import DEFAULT_CATALOGUE, ECHO +>>> entry = DEFAULT_CATALOGUE.lookup(ECHO) +>>> entry.project_name +'core-ops' +"""
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
cuprum/__init__.py(1 hunks)cuprum/catalogue.py(1 hunks)cuprum/program.py(1 hunks)cuprum/unittests/test_catalogue.py(1 hunks)docs/cuprum-design.md(1 hunks)docs/roadmap.md(1 hunks)docs/users-guide.md(1 hunks)pyproject.toml(1 hunks)tests/behaviour/test_catalogue_behaviour.py(1 hunks)tests/features/catalogue.feature(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.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
numpystyle guide. Use a single-line summary for private functions and methods, and full structured docs for all public interfaces.- Move conditionals with >2 branches to predicate/helper functions
- Avoid
eval,exec,pickle, monkey-patching,ctypes, unsafe shell- Every module must begin with a triple-quoted docstring explaining its purpose, utility, and usage, including example calls if appropriate.
- Comments and docs must follow en-GB-oxendict (-ize / -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 byFIXME:or a ticket link, and used only as a last resort.- Use
pytestfixtures for shared setup (conftest.pyorfixtures/)- Replace duplicate tests with
@pytest.mark.parametrize- Prefer
pytest-mockorunittest.mockfor stubs/mocks- Use
assert …, "message"over bare asserts- Reflect all API/behaviour changes in
docs/and update roadmap on completion- Files must not exceed 400 logical lines:
- Decompose large modules into subpackages
- Split large
match/caseor dispatch tables by domain and collocate with targets if appropriate- Move bulky data (fixtures, templates) to external files for parsing at runtime
- Mutable defaults and shadowed built-ins are forbidden
- All code must have clear type hints using modern style (
A | B,list[str],class Foo[A]:,type Bar = int, etc.), with ABC imports drawn from the correct stdlib module.- 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/catalogue.pycuprum/unittests/test_catalogue.pytests/behaviour/test_catalogue_behaviour.pycuprum/__init__.pycuprum/program.py
**/*.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/roadmap.mddocs/users-guide.mddocs/cuprum-design.md
🧬 Code graph analysis (3)
cuprum/unittests/test_catalogue.py (1)
cuprum/catalogue.py (7)
ProgramCatalogue(43-104)ProjectSettings(17-27)UnknownProgramError(12-13)allowlist(53-55)visible_settings(74-76)lookup(61-68)is_allowed(57-59)
tests/behaviour/test_catalogue_behaviour.py (1)
cuprum/catalogue.py (5)
ProgramCatalogue(43-104)ProjectSettings(17-27)UnknownProgramError(12-13)lookup(61-68)visible_settings(74-76)
cuprum/__init__.py (1)
cuprum/catalogue.py (4)
ProgramCatalogue(43-104)ProgramEntry(31-40)ProjectSettings(17-27)UnknownProgramError(12-13)
🪛 LanguageTool
docs/cuprum-design.md
[uncategorized] ~236-~236: Loose punctuation mark.
Context: ...ttingsrecord that holds: -programs: curated Program` values owned by the p...
(UNLIKELY_OPENING_PUNCTUATION)
🔍 Remote MCP Ref
Summary of Additional Context
Based on the available documentation and tools, here's the relevant context for reviewing this PR:
Python NewType Pattern
The Program = NewType("Program", str) approach used in this PR is a standard Python typing pattern that allows the static type checker to treat the new type as a distinct subtype while maintaining runtime identity with the base type. At runtime, Derived(some_value) does not create overhead beyond a regular function call, and some_value is Derived(some_value) is always true at runtime.
Key implications for review:
- The implementation provides compile-time safety without runtime performance penalties
- Values of type
strcannot be used in places expectingProgram, helping prevent logic errors with minimal runtime cost - This is the appropriate choice for the cuprum use case
pytest-bdd Testing Framework
The PR's addition of pytest-bdd as a dev dependency enables behavior-driven testing through pytest, which aligns with the new behavioral test files (tests/features/catalogue.feature and tests/behaviour/test_catalogue_behaviour.py) included in the PR.
Design Alignment
The PR implements a catalogue/allowlist pattern with:
- Strong typing via
NewTypefor program identifiers - Metadata coupling through
ProjectSettings(containing programs, documentation_locations, and noise_rules) - Default-deny security model via
UnknownProgramError - Read-only metadata exposure through
visible_settings()
This is a foundational safety layer that enforces command allowlisting at the type system level, complemented by runtime validation through exception handling.,
⏰ 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 (13)
docs/roadmap.md (1)
15-20: Keep typed command core task marked as completeThe checked task accurately reflects the implemented
ProgramNewType, curated catalogue, and tests, so no change is required.docs/cuprum-design.md (1)
231-246: Retain catalogue metadata description as writtenThe new subsection accurately describes
ProgramCatalogue,ProjectSettings, the default allowlist,UnknownProgramError, andvisible_settings()without over‑specifying implementation details, so no change is required.pyproject.toml (1)
11-18: Add pytest-bdd to dev dependencies to back behaviour testsThe
pytest-bddentry in the dev group matches the new BDD tests and is consistent with the existing dependency style; keep this addition.Run this script to confirm the dependency is wired through configuration and lockfile:
#!/bin/bash set -euo pipefail echo "Searching for pytest-bdd in pyproject.toml" rg -n "pytest-bdd" pyproject.toml || echo "pytest-bdd not declared in pyproject.toml" echo "Checking for pytest-bdd in uv.lock (if present)" if [ -f uv.lock ]; then rg -n "pytest-bdd" uv.lock || echo "pytest-bdd not found in uv.lock" else echo "uv.lock not present at repository root" fitests/features/catalogue.feature (1)
1-11: Retain BDD scenarios for blocking unknown programmes and exposing metadataThe feature file clearly specifies the default‑deny behaviour and metadata exposure contract for the catalogue, matching the behavioural tests; no change is required.
tests/behaviour/test_catalogue_behaviour.py (1)
1-77: Keep behaviour tests wiring for catalogue scenariosThe pytest‑bdd scenarios and step implementations correctly exercise unknown‑programme rejection and project metadata visibility via
visible_settings, with clear fixture and type usage; retain this structure.cuprum/__init__.py (1)
5-34: LGTM!Imports are well-organised, and
__all__is alphabetically sorted with all re-exported symbols correctly included.cuprum/catalogue.py (7)
3-9: LGTM!Imports are minimal and correctly organised. The
typalias fortypingis consistent.
12-13: LGTM!Inheriting from
LookupErroris semantically appropriate for "programme not found" scenarios.
16-27: LGTM!Frozen dataclass with slots is appropriate for immutable metadata. Using tuples for collection fields prevents mutation.
30-40: LGTM!Clean immutable dataclass with a convenient
project_nameproperty.
43-50: LGTM!Constructor correctly indexes projects and programmes, building the allowlist from the index keys.
114-127: LGTM!Default project definitions are well-structured with immutable tuples for programmes, documentation locations, and noise rules.
131-142: LGTM!Public exports are correctly scoped and alphabetically sorted. Private helper
_coerce_programis correctly excluded.
- Allow ProgramCatalogue methods to accept both Program and str types for convenience. - Added _coerce_program helper function to normalize input. - Updated is_allowed() and lookup() to support both types. - Improved ProjectSettings and ProgramCatalogue with cache and consistency checks. - Added tests for these improvements and public API coverage. - Updated documentation and examples to use top-level imports from cuprum. This enhancement improves ergonomics and type safety of program catalogue lookups and checks. Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
docs/users-guide.md (1)
5-20: Align narrative import guidance with the public API surfaceUpdate the narrative bullet so it matches the recommended top-level imports used in the examples and avoids binding readers to the internal
cataloguemodule path.-- Import curated programs from `cuprum.catalogue` (for example `ECHO`, `LS`). +- Import curated programs from `cuprum` (for example `ECHO`, `LS`).This keeps documentation aligned with the
cuprumpackage re‑exports.Also applies to: 24-40
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
cuprum/__init__.py(1 hunks)cuprum/catalogue.py(1 hunks)cuprum/program.py(1 hunks)cuprum/unittests/test_catalogue.py(1 hunks)cuprum/unittests/test_public_api.py(1 hunks)docs/users-guide.md(1 hunks)tests/behaviour/test_catalogue_behaviour.py(1 hunks)tests/features/catalogue.feature(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.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
**/*.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
numpystyle guide. Use a single-line summary for private functions and methods, and full structured docs for all public interfaces.- Move conditionals with >2 branches to predicate/helper functions
- Avoid
eval,exec,pickle, monkey-patching,ctypes, unsafe shell- Every module must begin with a triple-quoted docstring explaining its purpose, utility, and usage, including example calls if appropriate.
- Comments and docs must follow en-GB-oxendict (-ize / -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 byFIXME:or a ticket link, and used only as a last resort.- Use
pytestfixtures for shared setup (conftest.pyorfixtures/)- Replace duplicate tests with
@pytest.mark.parametrize- Prefer
pytest-mockorunittest.mockfor stubs/mocks- Use
assert …, "message"over bare asserts- Reflect all API/behaviour changes in
docs/and update roadmap on completion- Files must not exceed 400 logical lines:
- Decompose large modules into subpackages
- Split large
match/caseor dispatch tables by domain and collocate with targets if appropriate- Move bulky data (fixtures, templates) to external files for parsing at runtime
- Mutable defaults and shadowed built-ins are forbidden
- All code must have clear type hints using modern style (
A | B,list[str],class Foo[A]:,type Bar = int, etc.), with ABC imports drawn from the correct stdlib module.- 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/program.pycuprum/unittests/test_public_api.pytests/behaviour/test_catalogue_behaviour.pycuprum/catalogue.pycuprum/__init__.pycuprum/unittests/test_catalogue.py
🧬 Code graph analysis (3)
cuprum/unittests/test_public_api.py (1)
cuprum/catalogue.py (7)
ProgramCatalogue(56-119)ProgramEntry(44-53)ProjectSettings(30-40)UnknownProgramError(25-26)lookup(76-83)project_name(51-53)is_allowed(71-74)
tests/behaviour/test_catalogue_behaviour.py (1)
cuprum/catalogue.py (8)
ProgramCatalogue(56-119)ProgramEntry(44-53)ProjectSettings(30-40)UnknownProgramError(25-26)lookup(76-83)visible_settings(89-91)project_name(51-53)is_allowed(71-74)
cuprum/unittests/test_catalogue.py (1)
cuprum/catalogue.py (7)
ProgramCatalogue(56-119)ProjectSettings(30-40)UnknownProgramError(25-26)allowlist(67-69)visible_settings(89-91)is_allowed(71-74)lookup(76-83)
🔍 Remote MCP Ref
Summary of additional concrete facts found in the PR changes (relevant for review)
- Program is a nominal NewType alias: Program = NewType("Program", str).
- Catalogue normalization: _coerce_program(raw) returns Program(raw) (no transformation).
- Unknown programs raise UnknownProgramError from lookup(); lookup() uses the program→project map and raises with message "Program '' is not in the catalogue allowlist". visible_settings() returns an immutable MappingProxyType of projects. allowlist is a frozenset of Program values.
- Duplicate guards:
- _index_projects raises ValueError if a project name is registered more than once.
- _index_programs raises ValueError if the same Program is owned by multiple projects (message includes owner).
- ProjectSettings is a frozen dataclass with fields: name, programs (tuple[Program,...]), documentation_locations (tuple[str,...]), noise_rules (tuple[str,...]); has owns(program) helper. ProgramEntry is a frozen dataclass holding program and its ProjectSettings and exposes project_name property.
- Default constants and catalogue contents:
- CORE_OPS_PROJECT = "core-ops", DOCUMENTATION_PROJECT = "docs".
- ECHO = Program("echo"), LS = Program("ls"), DOC_TOOL = Program("mdbook").
- DEFAULT_PROJECTS contains:
- core-ops: programs (ECHO, LS), documentation_locations ("docs/users-guide.md#program-catalogue"), noise_rules (r"^progress:", r"^note:")
- docs: programs (DOC_TOOL), documentation_locations ("https://docs.example.invalid/cuprum/catalogue"), noise_rules (r"^[INFO]")
- DEFAULT_CATALOGUE = ProgramCatalogue(projects=DEFAULT_PROJECTS).
- cuprum/init.py re-exports: CORE_OPS_PROJECT, DEFAULT_CATALOGUE, DEFAULT_PROJECTS, DOCUMENTATION_PROJECT, DOC_TOOL, ECHO, LS, PACKAGE_NAME, Program, ProgramCatalogue, ProgramEntry, ProjectSettings, UnknownProgramError (i.e., catalogue + Program are publicly exposed at package level).
- PR page metadata (author branch/PR #/title) available at the PR URL.
Sources
- cuprum/catalogue.py (catalogue implementation, constants, defaults)
- cuprum/program.py (Program NewType)
- cuprum/init.py (public re-exports)
- PR page: #6 (PR metadata)
⏰ 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 (4)
cuprum/program.py (1)
1-17: Keep Program NewType and docstring as implementedRetain this nominal
Programalias and the concise example; the public surface and typing behaviour align with the catalogue usage and tests.tests/features/catalogue.feature (1)
1-17: Keep feature scenarios as writtenRetain these three scenarios; they map cleanly onto the step definitions and exercise the key safety and metadata behaviours of the catalogue.
cuprum/__init__.py (1)
1-12: Keep expanded package docstring and re-exportsRetain this package‑level documentation and the explicit
__all__; the public API surface for catalogue usage is clear and matches the tests and docs.Also applies to: 16-29, 33-47
cuprum/catalogue.py (1)
1-158: Approve catalogue core, safety checks, and defaultsKeep this implementation as-is: the coercion helper, duplicate guards, allowlist semantics, visible settings cache, and default projects all align with the intended safety guarantees and are well covered by the tests.
…ory messages Improved test clarity by adding detailed assertion messages in unit and behaviour tests for catalogue and public API. This includes more descriptive failure messages for duplicate project and program detection, public export availability, and behaviour verification in downstream services. Also fixed a docs import path. Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
leynos
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
visible_settings()Changes
Programas a NewType:Program = NewType("Program", str)UnknownProgramError,ProjectSettings,ProgramEntry, andProgramCatalogueCORE_OPS_PROJECT,ECHO,LS,DOC_TOOLDEFAULT_CATALOGUEwithDEFAULT_PROJECTSvisible_settings()to surface project metadata without mutationCORE_OPS_PROJECT,DEFAULT_CATALOGUE,DEFAULT_PROJECTS,DOC_TOOL,ECHO,LS,ProgramCatalogue,ProgramEntry,ProjectSettings,UnknownProgramError,Programcuprum/unittests/test_catalogue.pycover NewType behaviour, default allowlist, safety errors, and metadata exposuretests/behaviour/test_catalogue_behaviour.pyand feature filetests/features/catalogue.featureverify unknown programs are blocked and metadata is exposeddocs/cuprum-design.md: add section describing catalogue metadata and safety modeldocs/users-guide.md: explain how to use the curated catalogue and read metadatadocs/roadmap.md: reflect completion of the typed command core steppyproject.toml: addpytest-bddto dev dependenciesuv.lockupdated to reflect new/test dependenciesUsage example
Access the default safe catalogue and look up a program:
Unknown programs are rejected:
Why this matters
Testing
Next steps
🌿 Generated by Terry
ℹ️ Tag @terragon-labs to ask questions and address PR feedback
📎 Task: https://www.terragonlabs.com/task/c6f484cc-6f2b-418e-8025-9723e201d830
Summary by Sourcery
Introduce a typed program representation and a curated, metadata-rich catalogue that safely gates command execution via an allowlist.
New Features:
Build:
Documentation:
Tests: