Release Samsarix Routine Engine 0.2 - #2
Conversation
|
Warning Review limit reached
Next review available in: 44 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Summary by CodeRabbit
WalkthroughRoutine Engine 0.2 adds a versioned workflow schema, bounded validation, deterministic planning, resumable execution, checkpoint persistence, new CLI commands, release-readiness examples, benchmarks, and updated release documentation. ChangesRoutine Engine 0.2
Sequence Diagram(s)sequenceDiagram
participant CLI
participant RoutineEngine
participant release_actions
participant JsonStore
CLI->>RoutineEngine: run release-readiness workflow
RoutineEngine->>release_actions: validate files and compute hashes
release_actions-->>RoutineEngine: return release evidence
RoutineEngine->>JsonStore: persist checkpoints and final result
RoutineEngine-->>CLI: return run status and result
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 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: 12
🤖 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 `@benchmarks/benchmark_engine.py`:
- Line 36: Update the p95 calculation to use the nearest-rank index ceil(0.95 *
len(durations)) - 1 instead of flooring the percentile rank, while retaining the
existing lower-bound protection for valid indexing.
In `@CHANGELOG.md`:
- Around line 7-8: Align the 0.2.0 release state across CHANGELOG.md (lines 7-8
and 38-39) and ROADMAP.md (lines 9-27): either move the changelog entry under
Unreleased until v0.2.0 exists, or, if it remains released, create the v0.2.0
tag and update the roadmap’s release-candidate and package-upload status to
reflect publication; apply the corresponding consistency update to
docs/PRODUCTIZATION.md.
In `@examples/release_actions.py`:
- Around line 1-30: Establish one import contract for the release example:
update examples/release_actions.py lines 1-30 to be importable as an installed
package or explicitly source-tree-only; update README.md lines 81-88 to state
that my_actions must be installed or exposed through an explicit module path;
update docs/USE_CASES.md lines 10-11 to include PYTHONPATH=. or the installed
plugin module name.
In `@src/routine_engine/engine.py`:
- Around line 212-213: Update the per-step checkpoint path in
RoutineEngine.arun, including both completed and skipped steps, so
JsonStore.record_step is executed via asyncio.to_thread rather than
synchronously on the event loop. Preserve checkpoint ordering and completion
behavior while preventing blocking file I/O and fsync from stalling concurrent
tasks; do not change the store’s persistence format unless batching is already
supported.
- Around line 289-294: Update the sync-action execution path around
asyncio.to_thread in the engine’s execution method to use an executor sized
according to max_concurrency, preventing the default executor from limiting or
deadlocking concurrent actions; otherwise document the bound in
API_REFERENCE.md. Also review the exception handling around
validate_json_payload and ensure deterministic WorkflowValidationError
serialization failures are not incorrectly consumed as retryable errors unless
that behavior is explicitly intended.
- Around line 74-81: Update the planning loop around ready in plan() to detect
when no steps are schedulable: if ready is empty, raise the same explicit error
used by arun() for a "validated workflow became unschedulable" condition before
appending or updating complete. Preserve normal layer construction for non-empty
ready tuples.
- Around line 322-329: Apply _format_error to the parameter-resolution failure
in the step execution path, replacing the raw “ParameterResolutionError”
f-string with the bounded, newline-escaped formatting used for action failures.
Extend _format_error only as needed to preserve that prefix, and ensure every
persisted error string, including the failure around parameter resolution,
remains within MAX_ERROR_LENGTH so StepResult.from_dict accepts restored
checkpoints.
- Around line 161-163: Collapse the nested conditions in the run initialization
logic by combining the `_store is not None` and `resume_run_id is None` checks
into a single conditional before calling `_store.begin_run`.
- Line 111: Update the error message in RoutineEngine.resume() and the
corresponding overlong line in RoutineEngine.run() to satisfy the 110-character
Ruff E501 limit, using a shorter or split message while preserving the existing
guidance to use await aresume(...).
In `@src/routine_engine/models.py`:
- Around line 186-190: Update the schema_version validation in the workflow
model to require an actual integer value, rejecting floats and other numeric
types that merely compare equal to WORKFLOW_SCHEMA_VERSION while continuing to
reject booleans. Preserve the existing WorkflowValidationError message and
ensure only the validated integer reaches Workflow.schema_version and downstream
ExecutionPlan.schema_version.
In `@src/routine_engine/storage.py`:
- Around line 80-83: In the routine update method surrounding the status check
and `record["steps"]` assignment, validate that the running record’s `steps`
value is a mapping before indexing it; raise `StorageError` for any malformed
value so invalid persisted state does not produce `TypeError`. Preserve the
existing inactive-record error and normal step-update behavior.
In `@tests/test_scheduling.py`:
- Around line 200-203: Update InterruptingStore.record_step to accept the
exported StepResult type instead of object, import StepResult from
routine_engine, and remove the type: ignore[arg-type] suppression from the
super().record_step call.
🪄 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: a63f38af-0db6-40e4-aa32-be8cb7c61def
📒 Files selected for processing (23)
.github/workflows/ci.ymlCHANGELOG.mdREADME.mdROADMAP.mdSECURITY.mdbenchmarks/benchmark_engine.pydocs/API_REFERENCE.mddocs/COMPETITIVE_ANALYSIS.mddocs/PRODUCTIZATION.mddocs/USE_CASES.mdexamples/release-readiness.jsonexamples/release_actions.pypyproject.tomlschemas/workflow-v1.schema.jsonsrc/routine_engine/__init__.pysrc/routine_engine/cli.pysrc/routine_engine/engine.pysrc/routine_engine/models.pysrc/routine_engine/schemas/workflow-v1.schema.jsonsrc/routine_engine/storage.pytests/test_execution.pytests/test_scheduling.pytests/test_workflow.py
| durations.append(time.perf_counter() - started) | ||
|
|
||
| median = statistics.median(durations) | ||
| p95 = sorted(durations)[max(0, int(len(durations) * 0.95) - 1)] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use a ceiling rank for p95.
Line 36 floors the percentile rank. For run counts not divisible by 20, it reports a lower order statistic. For example, two runs select the minimum value. Use ceil(0.95 * n) - 1 for nearest-rank p95.
Proposed fix
import argparse
+import math
import statistics
import time
@@
- p95 = sorted(durations)[max(0, int(len(durations) * 0.95) - 1)]
+ p95 = sorted(durations)[math.ceil(len(durations) * 0.95) - 1]🤖 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 `@benchmarks/benchmark_engine.py` at line 36, Update the p95 calculation to use
the nearest-rank index ceil(0.95 * len(durations)) - 1 instead of flooring the
percentile rank, while retaining the existing lower-bound protection for valid
indexing.
| ## [0.2.0] - 2026-08-08 | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Tracked files of interest:\n'
git ls-files | rg '(^|/)(CHANGELOG|ROADMAP|PRODUCTIZATION)\.md$|(^|/)\.gitmodules$' || true
printf '\nCHANGELOG.md relevant sections:\n'
if [ -f CHANGELOG.md ]; then
nl -ba CHANGELOG.md | sed -n '1,80p'
else
printf 'CHANGELOG.md not found\n'
fi
printf '\nROADMAP.md relevant section:\n'
if [ -f ROADMAP.md ]; then
nl -ba ROADMAP.md | sed -n '1,80p'
else
printf 'ROADMAP.md not found\n'
fi
printf '\nPRODUCTIZATION.md relevant line:\n'
if [ -f docs/PRODUCTIZATION.md ]; then
nl -ba docs/PRODUCTIZATION.md | sed -n '80,98p'
else
printf 'docs/PRODUCTIZATION.md not found\n'
fi
printf '\nGit tag refs containing 0.1.0 or 0.2.0:\n'
git tag -l 'v?0*1*0' 'v?0*2*0' | sort || true
printf '\nRemote refs/tags containing 0.2.0:\n'
git ls-remote --tags origin '*/v0.2.0' '*/v0.2.0{}' '*/v0.2.0' 2>/dev/null | sort || true
git ls-remote origin refs/tags/v0.2.0 refs/tags/v0.2.0^{} 2>/dev/null || trueRepository: Deathcharge/routine-engine
Length of output: 311
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'CHANGELOG.md relevant sections:\n'
awk 'NR>=1 && NR<=80 { printf "%6d %s\n", NR, $0 }' CHANGELOG.md
printf '\nROADMAP.md relevant section:\n'
awk 'NR>=1 && NR<=80 { printf "%6d %s\n", NR, $0 }' ROADMAP.md
printf '\nPRODUCTIZATION.md relevant line:\n'
awk 'NR>=80 && NR<=98 { printf "%6d %s\n", NR, $0 }' docs/PRODUCTIZATION.md
printf '\nGit tag refs containing 0.1.0 or 0.2.0:\n'
git tag -l 'v?0*1*0' 'v?0*2*0' | sort || true
printf '\nLocal refs (tags only) matching v0.2.0:\n'
git for-each-ref --format='%(refname:short) %(objecttype)' refs/tags | grep -E '(^|/)v0\.2\.0$|v0\.2\.0\^' || true
printf '\nRemote refs/tags containing v0.2.0 from origin (if available):\n'
git ls-remote --tags origin '*/v0.2.0' '*/v0.2.0{}' '*/v0.2.0' 2>/dev/null | sort || true
git ls-remote origin refs/tags/v0.2.0 refs/tags/v0.2.0^{} 2>/dev/null || trueRepository: Deathcharge/routine-engine
Length of output: 8109
Align the 0.2 release state across release docs.
CHANGELOG.md reports 0.2.0 as released and links to v0.2.0, while no v0.2.0 tag exists. ROADMAP.md and docs/PRODUCTIZATION.md still say public release tagging and package upload remain release-candidate/owner steps.
- Keep the changelog entry under
Unreleased, or create thev0.2.0tag if this change is meant to be released as0.2.0. - If the changelog remains released, update
ROADMAP.mdanddocs/PRODUCTIZATION.mdso they do not contradict a published release state.
📍 Affects 2 files
CHANGELOG.md#L7-L8(this comment)CHANGELOG.md#L38-L39ROADMAP.md#L9-L27
🤖 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 `@CHANGELOG.md` around lines 7 - 8, Align the 0.2.0 release state across
CHANGELOG.md (lines 7-8 and 38-39) and ROADMAP.md (lines 9-27): either move the
changelog entry under Unreleased until v0.2.0 exists, or, if it remains
released, create the v0.2.0 tag and update the roadmap’s release-candidate and
package-upload status to reflect publication; apply the corresponding
consistency update to docs/PRODUCTIZATION.md.
| """Trusted actions for the release-readiness workflow example.""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import hashlib | ||
| from pathlib import Path | ||
| from typing import Any | ||
|
|
||
| from routine_engine import ActionContext, RoutineEngine | ||
|
|
||
|
|
||
| def require_files(context: ActionContext) -> dict[str, Any]: | ||
| files = [Path(str(item)) for item in context.params["paths"]] | ||
| missing = [path.as_posix() for path in files if not path.is_file()] | ||
| if missing: | ||
| raise FileNotFoundError(f"required release files are missing: {', '.join(missing)}") | ||
| return { | ||
| "files": [path.as_posix() for path in files], | ||
| "total_bytes": sum(path.stat().st_size for path in files), | ||
| } | ||
|
|
||
|
|
||
| def sha256(context: ActionContext) -> dict[str, str]: | ||
| path = Path(str(context.params["path"])) | ||
| return {"path": path.as_posix(), "sha256": hashlib.sha256(path.read_bytes()).hexdigest()} | ||
|
|
||
|
|
||
| def register(engine: RoutineEngine) -> None: | ||
| engine.register("release.require_files", require_files) | ||
| engine.register("release.sha256", sha256) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Establish one import contract for example plugins.
The quality pipeline failed because examples.release_actions was not importable. CI line 36 only succeeds because it adds PYTHONPATH=., but the documented commands omit that requirement. Make the plugin installable, or require the same explicit source-path setup in all test and documentation commands.
examples/release_actions.py#L1-L30: move this fixture to an importable package, or define it as a source-tree-only plugin with an explicit path requirement.README.md#L81-L88: state thatmy_actionsmust be installed or available through an explicit module path.docs/USE_CASES.md#L10-L11: addPYTHONPATH=.or use the installed plugin module name.
🧰 Tools
🪛 GitHub Actions: CI / 7_quality.txt
[error] 1-1: Plugin import failed during pytest --cov --cov-report=term-missing: No module named 'examples'. The validate command could not import plugin 'examples.release_actions'.
📍 Affects 3 files
examples/release_actions.py#L1-L30(this comment)README.md#L81-L88docs/USE_CASES.md#L10-L11
🤖 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_actions.py` around lines 1 - 30, Establish one import
contract for the release example: update examples/release_actions.py lines 1-30
to be importable as an installed package or explicitly source-tree-only; update
README.md lines 81-88 to state that my_actions must be installed or exposed
through an explicit module path; update docs/USE_CASES.md lines 10-11 to include
PYTHONPATH=. or the installed plugin module name.
Source: Pipeline failures
| while len(complete) < len(definition.steps): | ||
| ready = tuple( | ||
| PlanStep(step.id, step.action, step.needs) | ||
| for step in definition.steps | ||
| if step.id not in complete and set(step.needs) <= complete | ||
| ) | ||
| layers.append(ready) | ||
| complete.update(step.id for step in ready) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Guard the planning loop against an empty layer.
If ready is empty, complete does not grow and the while condition stays true. The loop then appends empty tuples forever and consumes memory until the process dies.
validate() returns a Workflow instance unchanged when the caller passes one:
definition = workflow if isinstance(workflow, Workflow) else Workflow.from_dict(workflow)Workflow is exported in routine_engine.__all__, so a caller can construct the dataclass directly and skip _validate_acyclic. A cyclic instance then hangs plan(). arun() already raises for the same condition with "validated workflow became unschedulable". Apply the same explicit guard here.
🐛 Proposed fix
while len(complete) < len(definition.steps):
ready = tuple(
PlanStep(step.id, step.action, step.needs)
for step in definition.steps
if step.id not in complete and set(step.needs) <= complete
)
+ if not ready:
+ raise WorkflowValidationError(f"workflow '{definition.id}' contains a dependency cycle")
layers.append(ready)
complete.update(step.id for step in ready)🤖 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/routine_engine/engine.py` around lines 74 - 81, Update the planning loop
around ready in plan() to detect when no steps are schedulable: if ready is
empty, raise the same explicit error used by arun() for a "validated workflow
became unschedulable" condition before appending or updating complete. Preserve
normal layer construction for non-empty ready tuples.
| asyncio.get_running_loop() | ||
| except RuntimeError: | ||
| return asyncio.run(self.aresume(run_id)) | ||
| raise RuntimeError("RoutineEngine.resume() cannot be called inside an event loop; use 'await aresume(...)'") |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Report source lines longer than the configured Ruff line-length.
rg -n '.{111,}' src/routine_engine/engine.py || echo "no lines exceed 110 characters"Repository: Deathcharge/routine-engine
Length of output: 526
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file with line numbers around engine.py == "
sed -n '95,150p' src/routine_engine/engine.py | cat -n -v | sed 's/^/ /'
echo
echo "== pyproject ruff line-length context =="
rg -n -C 3 'line-length|E501|ruff|lint' pyproject.toml || true
echo
echo "== exact line lengths == "
python3 - <<'PY'
from pathlib import Path
p=Path('src/routine_engine/engine.py')
for i,line in enumerate(p.read_text().splitlines(),1):
if 100 <= i <= 150:
print(f"{i}: {len(line)}: {line!r}")
PY
echo
echo "== ruff availability and lint output for selected files =="
if command -v ruff >/dev/null 2>&1; then
ruff --version
ruff check --select E501 src/routine_engine/engine.py pyproject.toml
else
echo "ruff not installed"
fiRepository: Deathcharge/routine-engine
Length of output: 9227
Keep RoutineEngine.resume() and RoutineEngine.run() under the 110-character limit.
src/routine_engine/engine.py:111 has 116 characters and triggers Ruff E501; move this error outside routine_engine or shorten/split the string so Ruff passes. Line 146 is also over the limit.
🤖 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/routine_engine/engine.py` at line 111, Update the error message in
RoutineEngine.resume() and the corresponding overlong line in
RoutineEngine.run() to satisfy the 110-character Ruff E501 limit, using a
shorter or split message while preserving the existing guidance to use await
aresume(...).
| if inspect.iscoroutinefunction(action): | ||
| value = await action(context) | ||
| else: | ||
| value = await asyncio.to_thread(action, context) | ||
| output = await value if inspect.isawaitable(value) else value | ||
| output = validate_json_payload(output, f"step '{step.id}' output", MAX_STEP_OUTPUT_BYTES) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
Sync actions depend on the default executor size, which is smaller than max_concurrency.
asyncio.to_thread uses the loop's default ThreadPoolExecutor. Its max_workers default is min(32, os.cpu_count() + 4). On a 1-CPU runner that is 5 threads, while max_concurrency allows up to 32. Blocking sync actions therefore serialize beyond the pool size. If two sync actions wait on each other, as tests/test_execution.py demonstrates with threading.Barrier, the run deadlocks once the ready set exceeds the pool size.
Document this bound in docs/API_REFERENCE.md, or give the engine its own executor sized from max_concurrency. The current test passes only because it uses two steps.
Also note that a non-JSON output raises WorkflowValidationError at line 294, which the except Exception at line 303 treats as a retryable failure. Confirm that consuming retries on a deterministic serialization error is intended.
🤖 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/routine_engine/engine.py` around lines 289 - 294, Update the sync-action
execution path around asyncio.to_thread in the engine’s execution method to use
an executor sized according to max_concurrency, preventing the default executor
from limiting or deadlocking concurrent actions; otherwise document the bound in
API_REFERENCE.md. Also review the exception handling around
validate_json_payload and ensure deterministic WorkflowValidationError
serialization failures are not incorrectly consumed as retryable errors unless
that behavior is explicitly intended.
| def _format_error(exc: Exception) -> str: | ||
| message = f"{type(exc).__name__}: {exc}".replace("\r", "\\r").replace("\n", "\\n") | ||
| if len(message) <= MAX_ERROR_LENGTH: | ||
| return message | ||
| suffix = "... [truncated]" | ||
| return message[: MAX_ERROR_LENGTH - len(suffix)] + suffix | ||
|
|
||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Apply _format_error to every persisted error string.
_format_error bounds only the action-failure path at line 304. The parameter-resolution failure at line 273 builds its error with a raw f-string:
error=f"ParameterResolutionError: {exc}",That value is neither newline-escaped nor truncated. StepResult.from_dict rejects any stored error longer than MAX_ERROR_LENGTH with "stored step error is malformed". arun constructs a StepResult for every stored step before it filters for SUCCESS, so one over-long resolution error makes the entire run fail to resume with "invalid checkpoint data". The producer and the restore validator must agree on the bound.
🐛 Proposed fix at line 273
- error=f"ParameterResolutionError: {exc}",
+ error=_format_error(exc, prefix="ParameterResolutionError"),Extend _format_error to accept an explicit prefix, or wrap the existing message:
def _format_error(exc: Exception) -> str:
message = f"{type(exc).__name__}: {exc}".replace("\r", "\\r").replace("\n", "\\n")
+ return _bound(message)
+
+
+def _bound(message: str) -> str:
if len(message) <= MAX_ERROR_LENGTH:
return message
suffix = "... [truncated]"
return message[: MAX_ERROR_LENGTH - len(suffix)] + suffix🤖 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/routine_engine/engine.py` around lines 322 - 329, Apply _format_error to
the parameter-resolution failure in the step execution path, replacing the raw
“ParameterResolutionError” f-string with the bounded, newline-escaped formatting
used for action failures. Extend _format_error only as needed to preserve that
prefix, and ensure every persisted error string, including the failure around
parameter resolution, remains within MAX_ERROR_LENGTH so StepResult.from_dict
accepts restored checkpoints.
| schema_version = detached.get("schema_version", WORKFLOW_SCHEMA_VERSION) | ||
| if isinstance(schema_version, bool) or schema_version != WORKFLOW_SCHEMA_VERSION: | ||
| raise WorkflowValidationError( | ||
| f"schema_version must be the integer {WORKFLOW_SCHEMA_VERSION}" | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject non-integer schema_version values.
The check compares with != only. 1.0 and Decimal-like numerics that equal 1 pass the check. The error text states "must be the integer 1", so the behavior contradicts the message. A float value is then stored on Workflow.schema_version, emitted by to_dict(), and copied into ExecutionPlan.schema_version, which is declared int.
🐛 Proposed fix
schema_version = detached.get("schema_version", WORKFLOW_SCHEMA_VERSION)
- if isinstance(schema_version, bool) or schema_version != WORKFLOW_SCHEMA_VERSION:
+ if (
+ isinstance(schema_version, bool)
+ or not isinstance(schema_version, int)
+ or schema_version != WORKFLOW_SCHEMA_VERSION
+ ):
raise WorkflowValidationError(
f"schema_version must be the integer {WORKFLOW_SCHEMA_VERSION}"
)📝 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.
| schema_version = detached.get("schema_version", WORKFLOW_SCHEMA_VERSION) | |
| if isinstance(schema_version, bool) or schema_version != WORKFLOW_SCHEMA_VERSION: | |
| raise WorkflowValidationError( | |
| f"schema_version must be the integer {WORKFLOW_SCHEMA_VERSION}" | |
| ) | |
| schema_version = detached.get("schema_version", WORKFLOW_SCHEMA_VERSION) | |
| if ( | |
| isinstance(schema_version, bool) | |
| or not isinstance(schema_version, int) | |
| or schema_version != WORKFLOW_SCHEMA_VERSION | |
| ): | |
| raise WorkflowValidationError( | |
| f"schema_version must be the integer {WORKFLOW_SCHEMA_VERSION}" | |
| ) |
🤖 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/routine_engine/models.py` around lines 186 - 190, Update the
schema_version validation in the workflow model to require an actual integer
value, rejecting floats and other numeric types that merely compare equal to
WORKFLOW_SCHEMA_VERSION while continuing to reject booleans. Preserve the
existing WorkflowValidationError message and ensure only the validated integer
reaches Workflow.schema_version and downstream ExecutionPlan.schema_version.
| if record.get("status") != "running": | ||
| raise StorageError(f"run '{run_id}' is not active") | ||
| record["steps"][step.step_id] = step.to_dict() | ||
| self._write(state) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Raise StorageError when a running record has a malformed steps value.
Line 82 indexes record["steps"] without a type check. If a state file contains a record with "status": "running" and a steps value that is not a mapping, the assignment raises TypeError. The class contract states that invalid persisted state raises StorageError, and cli.main catches only (RoutineEngineError, OSError, ValueError), so a TypeError escapes as an unhandled traceback.
🛡️ Proposed fix
if record.get("status") != "running":
raise StorageError(f"run '{run_id}' is not active")
+ if not isinstance(record.get("steps"), dict):
+ raise StorageError(f"run '{run_id}' has malformed checkpoint data")
record["steps"][step.step_id] = step.to_dict()📝 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.
| if record.get("status") != "running": | |
| raise StorageError(f"run '{run_id}' is not active") | |
| record["steps"][step.step_id] = step.to_dict() | |
| self._write(state) | |
| if record.get("status") != "running": | |
| raise StorageError(f"run '{run_id}' is not active") | |
| if not isinstance(record.get("steps"), dict): | |
| raise StorageError(f"run '{run_id}' has malformed checkpoint data") | |
| record["steps"][step.step_id] = step.to_dict() | |
| self._write(state) |
🤖 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/routine_engine/storage.py` around lines 80 - 83, In the routine update
method surrounding the status check and `record["steps"]` assignment, validate
that the running record’s `steps` value is a mapping before indexing it; raise
`StorageError` for any malformed value so invalid persisted state does not
produce `TypeError`. Preserve the existing inactive-record error and normal
step-update behavior.
| class InterruptingStore(JsonStore): | ||
| def record_step(self, run_id: str, step: object) -> None: | ||
| super().record_step(run_id, step) # type: ignore[arg-type] | ||
| raise RuntimeError("simulated process interruption") |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Type the override with StepResult and drop the suppression.
The override widens step to object, which forces the # type: ignore[arg-type] on the super() call. StepResult is exported from routine_engine. Use the real type so strict mypy checks the call instead of skipping it. A blanket suppression here hides a future signature change to record_step.
♻️ Proposed refactor
+from routine_engine import StepResult
+
class InterruptingStore(JsonStore):
- def record_step(self, run_id: str, step: object) -> None:
- super().record_step(run_id, step) # type: ignore[arg-type]
+ def record_step(self, run_id: str, step: StepResult) -> None:
+ super().record_step(run_id, step)
raise RuntimeError("simulated process interruption")🤖 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 `@tests/test_scheduling.py` around lines 200 - 203, Update
InterruptingStore.record_step to accept the exported StepResult type instead of
object, import StepResult from routine_engine, and remove the type:
ignore[arg-type] suppression from the super().record_step call.
|
Final exact-head release evidence: commit ed3ec48; all quality and Python 3.10-3.13 Ubuntu/Windows checks succeeded; CodeRabbit succeeded; final wheel SHA-256 ad98069be298cfbc9280fc6a7abc46065438a35eaa0fda186adf5e54a1cf5c65; final sdist SHA-256 93bc192c4b2aa8464017b0c65f83a3d242aaee549f644afbcd6ad67a3f36fd6f; both artifacts passed twine check. Rollback ref: archive/pre-routine-engine-0.2-2026-08-08. |
1 participant
Ships the independently usable 0.2 workflow contract: deterministic plans, explicit resource limits, non-blocking sync actions, atomic per-step checkpoints, crash-safe resume, run inspection CLI commands, bundled workflow v1 JSON Schema, a tested release-readiness consumer, and evidence-backed product/security documentation. Local gates: 48 tests pass; 91.42% branch coverage; Ruff, format, strict mypy, build, and Twine pass; installed-wheel CLI/schema/consumer smoke passes; Bandit reports no findings; pip-audit reports no known vulnerabilities after upgrading smoke-environment bootstrap tools. Benchmark (Python 3.11, 256 steps, 20 runs): 640.13 ms median / 849.61 ms p95. Exact head: ab2fa08. Wheel SHA-256: c9bfef314f941d5692a62ba47f84297fb5ca9d116acac9080a41de768018afe5. Sdist SHA-256: 604b7117f57b81a66d4aae004bc98607de510ee9d2c36912e22bac8a9822d574. Rollback ref: archive/pre-routine-engine-0.2-2026-08-08.