Add workflow contract regression suites - #9
Conversation
Summary by CodeRabbit
WalkthroughThe change adds validated workflow regression suites, suite execution and reporting APIs, a CLI ChangesWorkflow regression suites
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant load_suite
participant run_suite
participant Workflow
CLI->>load_suite: Load workflow and suite
load_suite-->>CLI: Return WorkflowSuite
CLI->>run_suite: Execute all cases
run_suite->>Workflow: Run each case input
Workflow-->>run_suite: Return output or execution error
run_suite-->>CLI: Return SuiteResult
CLI-->>CLI: Render report and exit status
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/release-policy.suite.json`:
- Around line 35-48: Add a regression case alongside “missing version is
rejected” in the release-policy suite using an empty or missing commit while
providing a valid version and approval; assert the error step_id is
require_commit and message_contains is “commit is required”.
In `@README.md`:
- Line 43: Update the release-policy example command in README.md to include the
Unix virtual-environment executable form `.venv/bin/samsarix-spirals`, or use an
activated-venv invocation, while preserving the documented Windows command and
existing platform-specific conventions.
- Around line 69-74: Update the README regression-suite description and
exit-status list to match the `test` command contract: status 1 covers
mismatched expected outputs or execution errors, including cases where workflow
execution succeeds, while status 2 covers suite validation errors as well as
workflow-structure errors. Replace the non-observable “contract has changed”
wording and ensure both sections consistently describe these statuses.
In `@src/samsarix_spirals/suite.py`:
- Around line 49-52: Update WorkflowSuite.from_dict to first validate and detach
the entire document through validate_json_value, enforcing the suite-wide JSON
and string limits before parsing fields. Continue using the detached validated
document for field extraction, and retain the existing suite-specific checks
such as _reject_unknown_keys and case validation.
- Around line 256-258: The output check in the case evaluation flow must use
JSON-type-aware recursive comparison instead of Python equality, distinguishing
booleans from numbers at every nesting level. Update the comparison around
CaseResult creation and add regression cases covering true versus 1 and false
versus 0.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1d428d9b-ea42-45e6-b3f2-4c362888ae1a
📒 Files selected for processing (16)
.github/workflows/ci.ymlCHANGELOG.mdREADME.mdROADMAP.mddocs/COMPETITIVE_POSITIONING.mddocs/WORKFLOW_FORMAT.mdexamples/release-policy.jsonexamples/release-policy.suite.jsonpyproject.tomlsrc/samsarix_spirals/__init__.pysrc/samsarix_spirals/cli.pysrc/samsarix_spirals/model.pysrc/samsarix_spirals/suite.pytests/test_cli.pytests/test_public_api.pytests/test_suite.py
| { | ||
| "name": "missing version is rejected", | ||
| "input": { | ||
| "version": "", | ||
| "commit": "abc123", | ||
| "approved": true | ||
| }, | ||
| "expect": { | ||
| "error": { | ||
| "step_id": "require_version", | ||
| "message_contains": "version is required" | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
Add a missing-commit regression case.
examples/release-policy.json lines 19-25 require commit. This suite does not assert that failure contract.
A change can remove or alter require_commit while this CI suite still passes. Add a case with an empty or missing commit value. Expect step_id require_commit and message_contains commit is required.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@examples/release-policy.suite.json` around lines 35 - 48, Add a regression
case alongside “missing version is rejected” in the release-policy suite using
an empty or missing commit while providing a valid version and approval; assert
the error step_id is require_commit and message_contains is “commit is
required”.
| ```console | ||
| .venv\Scripts\samsarix-spirals validate examples/hello.json | ||
| .venv\Scripts\samsarix-spirals run examples/hello.json --input examples/hello.input.json | ||
| .venv\Scripts\samsarix-spirals test examples/release-policy.json examples/release-policy.suite.json |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Add the Unix form of the new suite command.
The new example uses the Windows-only .venv\Scripts\samsarix-spirals path, although this README also documents macOS and Linux installation. Add the .venv/bin/samsarix-spirals form or show an activated-venv command. Otherwise Unix users cannot run the new release-policy example by copying the documented command.
This follows the platform-specific paths already documented in README.md.
Proposed documentation fix
- .venv\Scripts\samsarix-spirals test examples/release-policy.json examples/release-policy.suite.json
+# Windows
+.venv\Scripts\samsarix-spirals test examples/release-policy.json examples/release-policy.suite.json
+# macOS/Linux
+.venv/bin/samsarix-spirals test examples/release-policy.json examples/release-policy.suite.json📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .venv\Scripts\samsarix-spirals test examples/release-policy.json examples/release-policy.suite.json | |
| # Windows | |
| .venv\Scripts\samsarix-spirals test examples/release-policy.json examples/release-policy.suite.json | |
| # macOS/Linux | |
| .venv/bin/samsarix-spirals test examples/release-policy.json examples/release-policy.suite.json |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` at line 43, Update the release-policy example command in README.md
to include the Unix virtual-environment executable form
`.venv/bin/samsarix-spirals`, or use an activated-venv invocation, while
preserving the documented Windows command and existing platform-specific
conventions.
| ## Regression suites | ||
|
|
||
| A suite stores named inputs beside exact expected outputs or expected execution errors. | ||
| The `test` command runs every case, reports all mismatches, and exits `1` if the contract | ||
| has changed. Reports describe the mismatch without echoing input or output values, which | ||
| reduces accidental disclosure of fixture data in CI logs. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Update the failure-status documentation for test.
The test command returns 1 when a suite case does not match its expected output or error, even when workflow execution succeeds. The current sentence says 1 means the contract changed, which is not observable, and the later exit-status list describes 1 only as execution failure. Suite validation errors also return 2, but the list names only workflow structure. Align both sections with the CLI contract.
The test branch in src/samsarix_spirals/cli.py, Lines 97-143, and tests/test_cli.py, Lines 142-160, provide the behavior used here.
Proposed documentation fix
- The `test` command runs every case, reports all mismatches, and exits `1` if the contract
- has changed.
+ The `test` command runs every case, reports all mismatches, and exits `1` if a case does
+ not match its expected output or error.
- CLI exit `0`: validation or execution succeeded.
- CLI exit `1`: a valid workflow failed during execution, such as a false assertion.
- CLI exit `2`: arguments, files, JSON, or workflow structure were invalid.
+ CLI exit `0`: validation or execution succeeded, or all suite cases passed.
+ CLI exit `1`: execution failed, or a suite case did not match.
+ CLI exit `2`: arguments, files, JSON, workflow, or suite structure were invalid.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` around lines 69 - 74, Update the README regression-suite
description and exit-status list to match the `test` command contract: status 1
covers mismatched expected outputs or execution errors, including cases where
workflow execution succeeds, while status 2 covers suite validation errors as
well as workflow-structure errors. Replace the non-observable “contract has
changed” wording and ensure both sections consistently describe these statuses.
| def from_dict(cls, document: Mapping[str, object]) -> WorkflowSuite: | ||
| """Validate and detach a regression-suite mapping.""" | ||
| issues: list[str] = [] | ||
| _reject_unknown_keys(document, {"suite_version", "name", "cases"}, "$", issues) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Apply JSON limits to the complete suite document.
Line 49 validates input and expect.output values separately. It does not validate document as one JSON tree.
A suite can exceed the documented total-value limit across cases. expect.error.step_id and expect.error.message_contains also bypass the documented string-length limit.
Validate and detach the complete document with validate_json_value before field parsing. Retain the suite-specific field checks.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/samsarix_spirals/suite.py` around lines 49 - 52, Update
WorkflowSuite.from_dict to first validate and detach the entire document through
validate_json_value, enforcing the suite-wide JSON and string limits before
parsing fields. Continue using the detached validated document for field
extraction, and retain the existing suite-specific checks such as
_reject_unknown_keys and case validation.
| if result.output != case.expected_output: | ||
| return CaseResult(case.name, False, "workflow output did not equal expected output") | ||
| return CaseResult(case.name, True) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use JSON-type-aware output comparison.
Line 256 uses Python equality. Python considers True == 1 and False == 0.
An output of 1 can therefore pass an expectation of true. This violates the exact JSON equality contract. Use a recursive comparator that distinguishes JSON booleans from JSON numbers. Add regression cases for true versus 1 and false versus 0.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/samsarix_spirals/suite.py` around lines 256 - 258, The output check in
the case evaluation flow must use JSON-type-aware recursive comparison instead
of Python equality, distinguishing booleans from numbers at every nesting level.
Update the comparison around CaseResult creation and add regression cases
covering true versus 1 and false versus 0.
1 participant
Summary
samsarix-spirals testCLI command with human and JSON reportsWhy
Samsarix Spirals needs a concrete use case that is meaningfully safer and more reviewable than an ad hoc script without competing with mature durable orchestrators. Checked-in workflow contracts provide that wedge for release gates, agent-generated structured data, and repository policy fixtures while preserving the no-network, no-code-execution runtime boundary.
Impact
Users can now keep named inputs beside expected outputs or expected execution failures and run every case locally or in CI. Reports intentionally omit fixture values to reduce accidental disclosure in logs. Existing workflow schema version 1 and existing commands remain compatible.
Verification
The local pip-audit client stalled without output; the existing hosted CI
pip-audit .step is the authoritative dependency-audit gate for this PR.