feat(ci): enforce changed-scope behavior mutation - #287
Conversation
|
Warning Review limit reached
Next review available in: 40 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: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (19)
📝 WalkthroughWalkthroughThe PR replaces the observational mutation pilot with a protected, claim-driven blocking gate. It adds callable ownership discovery, generated mutation configuration, fail-closed outcome classification, protected CI execution, expanded tests, documentation, dependency custody, and updated initiative records. ChangesBehavior Mutation Gate
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PullRequest
participant GitHubActions
participant ProtectedEvaluator
participant DisposableBackendEnvironment
participant Mutmut
PullRequest->>GitHubActions: trigger behavior mutation gate
GitHubActions->>ProtectedEvaluator: retrieve base evaluator and discover applicability
ProtectedEvaluator->>GitHubActions: return selection
GitHubActions->>DisposableBackendEnvironment: install locked mutation dependencies
GitHubActions->>ProtectedEvaluator: execute selected policy with enforcement
ProtectedEvaluator->>Mutmut: run generated mutation configuration
Mutmut->>ProtectedEvaluator: return mutation outcomes
ProtectedEvaluator->>GitHubActions: write verdict and evidence artifacts
Possibly related PRs
Suggested reviewers: 🚥 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: 4
🧹 Nitpick comments (4)
backend/tests/test_mutation_policy.py (2)
404-416: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert that non-mutmut TOML content survives the rewrite.
_write_mutmut_configis documented to preserve every table other than[tool.mutmut]. This test reads only["tool"]["mutmut"], so a regression that drops the retained[project]table would still pass.The fixture already writes
[project]at line 400. Assert it is still present.💚 Proposed addition
digest = _write_mutmut_config(backend, selection) - rendered = tomllib.loads(pyproject.read_text(encoding="utf-8"))["tool"]["mutmut"] + document = tomllib.loads(pyproject.read_text(encoding="utf-8")) + assert document["project"] == {"name": "example", "version": "0.1.0"} + rendered = document["tool"]["mutmut"]🤖 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 `@backend/tests/test_mutation_policy.py` around lines 404 - 416, Update the _write_mutmut_config test to verify that rewriting the mutmut section preserves the existing non-mutmut TOML tables, especially the [project] table already written by the fixture. Keep the current assertions on tomllib.loads(... )["tool"]["mutmut"], and add a check against the parsed pyproject content that [project] is still present after _write_mutmut_config runs.
505-544: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a nested-function case, and separate the two fail-closed guards.
Two coverage gaps.
First,
test_changed_callable_mapping_covers_decorated_async_and_nested_methodscovers only a decorated async method inside a class. It does not cover a function defined inside another function..ci/behavior-claims/WS-QUAL-001-05M.jsonclaimsscripts.mutation_policy._callable_spans.visit, which is exactly that shape. Add a case that changes only the inner function body and asserts the derived owner is the inner qualified name, not the enclosing one.Second,
test_module_level_and_deleted_callable_changes_fail_closedreplacesdef claimed()withSETTING = Truein one write. That triggerscurrent_unmappedandremovedat the same time, and both raiseunmappable_changed_logic. The test cannot prove that the removed-callable guard works on its own. Split it into one case that adds a module-level assignment while keeping every callable, and one case that deletes a callable while changing nothing else.New or materially changed backend subsystems must maintain at least 90% test coverage.
💚 Proposed nested-function case
+ def test_changed_callable_mapping_covers_functions_nested_in_functions(self) -> None: + with tempfile.TemporaryDirectory() as temporary: + root = Path(temporary) + self._initialize(root) + target = root / "backend/scripts/claimed.py" + body = "def outer():\n def inner():\n return {0}\n return inner\n" + target.write_text(body.format("True"), encoding="utf-8") + self._git(root, "add", ".") + self._git(root, "commit", "-m", "nested base") + self.base = self._git(root, "rev-parse", "HEAD") + target.write_text(body.format("False"), encoding="utf-8") + self._git(root, "add", ".") + self._git(root, "commit", "-m", "nested change") + head = self._git(root, "rev-parse", "HEAD") + assert changed_callables(root, self.base, head, "backend/scripts/claimed.py") == [ + "scripts.claimed.outer.inner" + ]🤖 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 `@backend/tests/test_mutation_policy.py` around lines 505 - 544, Add a test alongside test_changed_callable_mapping_covers_decorated_async_and_nested_methods that defines an outer function containing an inner function, changes only the inner body between commits, and asserts changed_callables returns the inner qualified name (including the enclosing function) rather than the outer name. Split test_module_level_and_deleted_callable_changes_fail_closed into independent cases: one preserves all callables while adding a module-level assignment to exercise current_unmapped, and the other removes a callable without unrelated changes to exercise the removed-callable guard; keep each case asserting unmappable_changed_logic.Source: Coding guidelines
backend/scripts/mutation_policy.py (2)
596-604: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winRequire
targeton owner records instead of falling back torsplit.The fallback at lines 602-603 derives the module by removing the last dotted segment of the callable name. For a method such as
scripts.mutation_policy.Service.claimed, that produces the modulescripts.mutation_policy.Serviceand the relative nameclaimed. The resulting globscripts.mutation_policy.Service.x_claimed__mutmut_*matches no mutant, so the callable is never mutated and the gate still reportspass.Every selection built by
build_selectionsetstarget. Only the test fixture atbackend/tests/test_mutation_policy.pylines 450-465 omits it. Make the absence a typed error and update that fixture.♻️ Proposed change
for owner in selection["target_owners"]: + target = owner.get("target") + if not target: + raise MutationPolicyError("missing_owner_target") + target_module = target.removeprefix("backend/").removesuffix(".py").replace("/", ".") for callable_name in owner["callables"]: - target_module = ( - owner.get("target", "") - .removeprefix("backend/") - .removesuffix(".py") - .replace("/", ".") - ) - if not target_module: - target_module = callable_name.rsplit(".", 1)[0] relative = callable_name.removeprefix(f"{target_module}.")🤖 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 `@backend/scripts/mutation_policy.py` around lines 596 - 604, Require every owner record processed by the target-module logic in build_selection to provide a non-empty target; remove the callable_name.rsplit fallback and raise the established typed error when target is absent. Update the owner fixture in test_mutation_policy.py to include the appropriate target value so it remains valid.
467-508: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the owner record into a helper and drop the redundant dict unpacking.
The
**({...})wrapper around a dict literal has no effect. The nested conditional expressions inside foursorted(...)calls repeat the sameblocking_policy and target == CALIBRATION_TARGETguard.A small helper reduces the nesting and states the calibration augmentation once.
♻️ Proposed refactor
+def _owner_record( + target: str, + claim: dict[str, Any], + *, + calibrated: bool, +) -> dict[str, Any]: + extra_callables = set(CALIBRATION_CALLABLES) if calibrated else set() + extra_tests = set(CALIBRATION_TESTS) if calibrated else set() + extra_outcomes = {"return"} if calibrated else set() + return { + "target": target, + "callables": sorted(set(claim.get("callables", [])) | extra_callables), + "tests": sorted(set(claim.get("tests", [])) | extra_tests), + "outcomes": sorted(set(claim.get("outcomes", [])) | extra_outcomes), + "boundaries": claim.get("boundaries", []), + }Then build each entry with
_owner_record(...)plus theselection_reasonkey.🤖 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 `@backend/scripts/mutation_policy.py` around lines 467 - 508, Extract the owner-data construction from the target comprehension into a helper such as _owner_record, passing the target, claims_by_target, and calibration-policy state it needs. In that helper, compute the blocking calibration condition once and apply it to the callables, tests, and outcomes augmentations, while preserving boundaries and sorting behavior. Replace the redundant **({...}) wrapper in the target_owners construction with the helper result plus the existing selection_reason field.
🤖 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 `@backend/scripts/mutation_policy.py`:
- Around line 245-249: Update the ClassDef handling in mutation_policy.py so the
class header line is always added to executable spans, even for undecorated,
base-free classes like the ClassDef branch. Keep the existing
decorator/base/keyword span logic, but append an additional span for the class
header unconditionally so _map_changed_lines can classify changes on plain class
declarations instead of silently passing them through changed_callables.
- Around line 431-439: Replace the fragile '"--discover"' source substring
checks in the blocking_policy and bootstrap logic with a dedicated
POLICY_CAPABILITY_MARKER emitted in a module-level comment or docstring, and
probe for that marker in base_policy. Preserve the one-shot WS-QUAL-001-05M
bootstrap behavior, and add a follow-up note to remove that chunk_id special
case after merge.
- Around line 551-577: Update the generated-TOML validation in the function
containing the rendered config and _sha256 call: wrap
tomllib.loads(rendered)["tool"]["mutmut"] in a tomllib.TOMLDecodeError handler
and raise the module’s typed MutationPolicyError so _main emits
mutation_policy_error instead of a traceback. Also replace the line-based
skipping rewrite with a parsed TOML rewrite that removes the existing
tool.mutmut configuration while preserving unrelated sections, then appends the
generated config and computes the digest from the parsed mutmut table.
- Around line 440-446: Handle deleted eligible targets before constructing or
consuming changed_targets: filter out deleted Python targets or assign them an
explicit deletion outcome so they cannot trigger missing_claim_target or
changed_target_without_behavior_claim. Update the target-discovery flow that
feeds derived_callables and preserve existing behavior for files still present
at HEAD.
---
Nitpick comments:
In `@backend/scripts/mutation_policy.py`:
- Around line 596-604: Require every owner record processed by the target-module
logic in build_selection to provide a non-empty target; remove the
callable_name.rsplit fallback and raise the established typed error when target
is absent. Update the owner fixture in test_mutation_policy.py to include the
appropriate target value so it remains valid.
- Around line 467-508: Extract the owner-data construction from the target
comprehension into a helper such as _owner_record, passing the target,
claims_by_target, and calibration-policy state it needs. In that helper, compute
the blocking calibration condition once and apply it to the callables, tests,
and outcomes augmentations, while preserving boundaries and sorting behavior.
Replace the redundant **({...}) wrapper in the target_owners construction with
the helper result plus the existing selection_reason field.
In `@backend/tests/test_mutation_policy.py`:
- Around line 404-416: Update the _write_mutmut_config test to verify that
rewriting the mutmut section preserves the existing non-mutmut TOML tables,
especially the [project] table already written by the fixture. Keep the current
assertions on tomllib.loads(... )["tool"]["mutmut"], and add a check against the
parsed pyproject content that [project] is still present after
_write_mutmut_config runs.
- Around line 505-544: Add a test alongside
test_changed_callable_mapping_covers_decorated_async_and_nested_methods that
defines an outer function containing an inner function, changes only the inner
body between commits, and asserts changed_callables returns the inner qualified
name (including the enclosing function) rather than the outer name. Split
test_module_level_and_deleted_callable_changes_fail_closed into independent
cases: one preserves all callables while adding a module-level assignment to
exercise current_unmapped, and the other removes a callable without unrelated
changes to exercise the removed-callable guard; keep each case asserting
unmappable_changed_logic.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 707541a9-4505-4568-9a54-5a2b012ec97c
📒 Files selected for processing (18)
.agent-loop/initiatives/WS-QUAL-001-backend-coverage-floor/CHUNK_MAP.md.agent-loop/initiatives/WS-QUAL-001-backend-coverage-floor/STATUS.md.agent-loop/initiatives/WS-QUAL-001-backend-coverage-floor/chunks/WS-QUAL-001-05M-blocking-behavior-mutation-gate.md.agent-loop/initiatives/WS-QUAL-001-backend-coverage-floor/reviews/WS-QUAL-001-05M-internal-review-evidence.md.agent-loop/initiatives/WS-QUAL-001-backend-coverage-floor/reviews/WS-QUAL-001-05M-pr-trust-bundle.md.ci/behavior-claims/README.md.ci/behavior-claims/WS-QUAL-001-05M.json.ci/behavior-claims/example.behavior-claim.json.github/workflows/mutation-pilot.ymlCONTRIBUTING.mdbackend/pyproject.tomlbackend/scripts/mutation_policy.pybackend/tests/test_mutation_policy.pydocs/operations_backend_testing.mdscripts/behavior-claim.schema.jsonscripts/mutation-requirements.inscripts/mutation-requirements.txtscripts/test_lightweight_agent_gates.py
💤 Files with no reviewable changes (1)
- backend/pyproject.toml
12bb1d9 to
722be2c
Compare
722be2c to
ae0f7ba
Compare
2 participants
WS-QUAL-001-05M PR Trust Bundle
Chunk
WS-QUAL-001-05M— Blocking Changed-Scope Behavior Mutation Gate (L1).Goal and human-approved intent
Turn the accepted
04Mobservational calibration into a bounded required gate:eligible changed behavior must have exact ownership, owning tests, and no
meaningful surviving selected mutant. The user explicitly accepted calibration
and started this chunk.
What changed and why
disposable configuration.
fail-closed status reconciliation.
pilotcheck always emit and block meaningful survivors.at protected base after the explicit one-time bootstrap.
interpretation, focused tests, and 90-percent subsystem coverage proof.
Coverage alone does not prove assertions detect wrong behavior. This gate adds
that proof only for the changed, explicitly owned callable scope; it does not
introduce a global mutation score.
Design chosen and alternatives rejected
The protected evaluator derives every eligible changed target and callable.
One exact claim supplies owning tests, observable outcomes, and real boundaries
but cannot narrow the derived scope. Mutmut runs in an archived disposable tree
with a hash-locked runner and runtime/test dependencies resolved from the
protected backend lock. Exact policy-owned strong/weak controls calibrate the
engine. All meaningful selected survivors and unsafe/incomplete statuses block.
Rejected: full-repository mutation, score thresholds, PR-head evaluator or
dependencies, free-form survivor exemptions, workflow path filters, and
running candidate tests with repository/CI credentials.
Scope control and product behavior
No application behavior, migration, Workstream product lifecycle,
authorization, payment, reputation, Backend lane, or coverage threshold
changed. Review decisions remain
accept,needs_revision, andreject;mutation findings are engineering evidence only. Existing global 78-percent
and protected 90-percent floors remain blocking.
Acceptance proof and test delta
05M, one claim, exact changed-callable ownership.now have behavioral regression tests.
CI integrity and reviewer results
The workflow retains stable job id
pilot, read-only permissions, pinnedActions, no path filters, preflight before installation, protected-base
authority, 15-minute job/720-second command/700-second engine limits, and
exact-head evidence. Architecture, senior engineering, QA, security,
product/ops, CI integrity, docs, reuse/dedup, and test-delta tracks pass after
valid findings were fixed. Detailed results are in the internal review evidence.
External review
GitHub Actions and CodeRabbit are pending publication and must pass on the exact
PR head. The bootstrap PR is nonblocking for mutation outcomes because merged
04Mlacks the new evaluator and lockeduvrunner; the first protected-mainrun establishes
05Mauthority for later PRs.Remaining risks and follow-up
Hosted dependency/runtime timing must remain within the accepted limits. The
stable internal job id
pilotis retained to avoid breaking required-checkconfiguration. Shared diff-line parsing may be deduplicated later; neither is a
merge blocker. Do not start another QUAL chunk from this PR.
Human review focus and merge ownership
selection.json,executed-selection.json, andevidence.json, especially calibration andverdict.blockers.exact final head.
Only the user may approve and merge this specific PR.
Summary by CodeRabbit
New Features
Documentation
Tests