Kill mutation survivors in the post-turn quality stop hook (#36, #37, #38, #40) - #50
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Summary
WalkthroughThe subprocess wrapper was simplified, while tests now verify exact Git commands, error messages, fallback process fields, output handling, formatting, category detection, and environment parsing. ChangesQuality hook coverage
Possibly related issues
Suggested labels: 🚥 Pre-merge checks | ✅ 19 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (19 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tighten each assertion, let exact truths shine, Comment |
Reviewer's GuideThis PR adds high-precision tests around the post-turn quality stop hook to kill/suppress surviving mutation-testing mutants, and performs small refactorings in the hook implementation to structurally suppress known equivalent mutants without changing behaviour. Flow diagram for updated parse_env default handlingflowchart TD
parse_env[parse_env]
flag_default["flag_default = '' (no mutate)"]
max_output_default["max_output_default = '12000' (no mutate)"]
parse_env --> flag_default
parse_env --> max_output_default
base_ref_env["os.environ.get POST_TURN_BASE_REF"]
always_fetch_env["os.environ.get POST_TURN_ALWAYS_FETCH"]
max_output_env["os.environ.get POST_TURN_MAX_OUTPUT_CHARS"]
compush_env["os.environ.get POST_TURN_COMPUSH"]
parse_env --> base_ref_env
parse_env --> always_fetch_env
parse_env --> max_output_env
parse_env --> compush_env
always_fetch_env --> parse_bool_env
compush_env --> parse_bool_env
max_output_env --> parse_max_output
flag_default --> always_fetch_env
flag_default --> compush_env
max_output_default --> max_output_env
parse_bool_env[parse_bool_env]
parse_max_output[parse_max_output]
parse_bool_env --> always_fetch
parse_bool_env --> compush
parse_max_output --> max_out
base_ref_env --> base_ref
base_ref[base_ref]
always_fetch[always_fetch]
max_out[max_out]
compush[compush]
base_ref --> result_tuple
always_fetch --> result_tuple
max_out --> result_tuple
compush --> result_tuple
result_tuple[("(base_ref, always_fetch, max_out, compush)")]
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
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 `@hooks/post-turn-quality-stop-hook.py`:
- Around line 151-157: Update the subprocess.run call in the hook’s command
execution helper to restore explicit check=False, and add the existing # pragma:
no mutate annotation used by parse_env() to suppress the mutation warning.
Preserve the current controlled-command noqa and non-zero return-code behavior.
- Around line 887-896: Update the max_output_default value used by
parse_max_output in the surrounding configuration setup to an unparsable
sentinel instead of duplicating "12000". Preserve parse_max_output's existing
fallback so it remains the single source of truth for the default maximum
output.
In `@hooks/test_post_turn_quality_stop_hook.py`:
- Around line 533-587: Move test_real_command_captures_text_output,
test_file_as_cwd_returns_error, and
test_filename_less_file_not_found_uses_fallback from TestGetMakeTargets into
TestRunOSError, placing them near test_nonexistent_cwd_returns_error. Preserve
each test’s implementation and assertions unchanged.
- Around line 771-772: Parenthesize the implicit two-part string concatenations
in the list literals at both occurrences, including the entries near the
relevant test messages, so Ruff ISC004 is satisfied and the expected output
remains unchanged.
- Around line 589-593: Split hooks/test_post_turn_quality_stop_hook.py into
focused modules using the existing test_run.py, test_helpers.py,
test_formatting.py, and test_env.py banner sections as boundaries, keeping each
module under 400 logical lines. Move the shared _completed, REPO, and ENV_VARS
setup into conftest.py or a fixtures module, then update imports and references
so all tests retain their current behavior.
🪄 Autofix (Beta)
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: 63f908cd-e37a-4313-80ea-de6878b440fe
📒 Files selected for processing (2)
hooks/post-turn-quality-stop-hook.pyhooks/test_post_turn_quality_stop_hook.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
leynos/cuprum(auto-detected)
|
@coderabbitai the post turn quality stop hook in this repo is obsolete and has been superseded by https://github.com/leynos/post-turn-quality-stop-hook/ Please provide an agent prompt for removal of the hook, installation scripts and tests, and the addition of a user's guide note directing consumers of the repo to the new hook. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '== candidate files =='
fd -HI -t f . | sort | sed -n '1,240p'
printf '%s\n' '== post-turn hook references =='
rg -n -i -C 2 'post[- ]turn|quality[- ]stop|stop hook|POST_TURN' . \
-g '!**/.git/**' || true
printf '%s\n' '== repository documentation candidates =='
fd -HI -t f '^(README|CONTRIBUTING|USER.*GUIDE|GUIDE|.*install.*|.*hook.*)\.' . \
-E .git | sort || trueLength of output: 15166
✏️ Learnings added
You are interacting with an AI system. |
Add targeted unit tests for the highest-value mutation-testing survivors reported against the stop hook: exact rendering tests for format_reason, boundary and split tests for truncate, representative parsing tests for parse_make_targets and is_missing_makefile, extension-mapping tests for detect_categories, full-tuple parse_env and parse_max_output tests, equality tests for default_categories, real-subprocess and error-branch tests for run(), and argv, wiring, and exact-message assertions for the git plumbing helpers. Annotate the four equivalent mutants (subprocess check=False and the falsy env-var defaults) with pragma no mutate comments, splitting the affected arguments onto their own lines so the suppression does not shield killable mutants on the same line.
mutmut 3.6 only honours pragma comments at statement level, so the argument-level annotations added earlier were ignored. Hoist the falsy env-var defaults into their own pragma-suppressed statements, drop the redundant check=False argument from run() (False is the subprocess default, so both of its mutants were equivalent by construction), and suppress the max-output default whose mutation falls back to the same 12000 value. Also convert the remaining substring assertions on git error messages to exact equality, cover truncate at max_chars=1, extend the make -qp fixture with a mid-stream blank line and a mixed pattern/plain rule, and exercise format_reason with a command entry lacking cmd/stdout/stderr keys and with a falsy base ref.
The replacement project lives at https://github.com/leynos/post-turn-quality-stop-hook. - Delete hooks/post-turn-quality-stop-hook.py and its test. - Drop the stop-hook install/check logic from install-hooks. - Remove the test-hooks Make target and hook-test plumbing. - Remove the mutation-testing workflow and its contract test. - Point user docs to the replacement repository.
Archive the VT Code session working state (.vtcode) created while removing the obsolete post-turn quality stop hook: task checklist, tool output spools, and tool policy snapshot.
Archive additional VT Code tool output spools generated while committing and pushing the post-turn hook removal.
e8cde16 to
5f79522
Compare
|
@coderabbitai Have the following now been resolved? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '== branch and changed paths =='
git status --short
git diff --name-status HEAD^ HEAD || true
printf '%s\n' '== candidate file presence =='
for path in hooks/post-turn-quality-stop-hook.py hooks/test_post_turn_quality_stop_hook.py install-hooks Makefile README.md docs/users-guide.md; do
if test -e "$path"; then
printf 'PRESENT %s\n' "$path"
else
printf 'ABSENT %s\n' "$path"
fi
done
printf '%s\n' '== outlines of relevant existing files =='
for path in install-hooks Makefile README.md docs/users-guide.md; do
if test -f "$path"; then
printf '\n-- %s --\n' "$path"
ast-grep outline "$path" --items all 2>/dev/null || true
fi
done
printf '%s\n' '== obsolete hook references (excluding Git metadata) =='
rg -n -i -C 2 \
-e 'post[- ]turn[- ]quality[- ]stop[- ]hook' \
-e 'POST_TURN_' \
-e '\btest-hooks\b' \
-e 'post[- ]turn hook' \
-e 'quality stop hook' \
. -g '!**/.git/**' || true
printf '%s\n' '== replacement URL references (excluding Git metadata) =='
rg -n -C 2 'https://github\.com/leynos/post-turn-quality-stop-hook' . -g '!**/.git/**' || true
printf '%s\n' '== tracked CI and workflow files =='
fd -HI -t f .github . 2>/dev/null | rg '(^|/)(workflows/.*|[^/]*\.(ya?ml|json|toml))$' | sort || trueLength of output: 5458 🏁 Script executed: #!/bin/bash
set -euo pipefail
gh pr view 50 --repo leynos/agent-helper-scripts \
--json state,mergeStateStatus,headRefName,statusCheckRollup,files \
--jq '{
state,
mergeStateStatus,
headRefName,
files: [.files[].path],
checks: [.statusCheckRollup[] | {
name: .name,
status: .status,
conclusion: .conclusion,
workflowName: .workflowName
}]
}'Length of output: 6802 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '== installer references and full installer =='
rg -n -i -C 3 'hook|claude|post[- ]turn|quality|POST_TURN|test-hooks' install-hooks || true
cat -n install-hooks
printf '%s\n' '== Makefile references and full Makefile =='
rg -n -i -C 3 'hook|post[- ]turn|quality|POST_TURN|test-hooks|test|ci' Makefile || true
cat -n Makefile
printf '%s\n' '== migration documentation ranges =='
sed -n '20,36p' README.md
sed -n '45,60p' docs/users-guide.md
sed -n '218,233p' docs/developers-guide.md
printf '%s\n' '== tracked path search, including ignored files =='
git ls-files --cached --others --exclude-standard | \
rg -n -i '(^|/)(post-turn-quality-stop-hook\.py|test_post_turn_quality_stop_hook\.py)$' || trueLength of output: 12412 🏁 Script executed: #!/bin/bash
set -euo pipefail
gh api --paginate \
'/repos/leynos/agent-helper-scripts/pulls/50/files?per_page=100' \
--jq '.[] | {
filename,
status,
additions,
deletions,
previous_filename,
patch: (if .filename == "hooks/post-turn-quality-stop-hook.py" or .filename == "hooks/test_post_turn_quality_stop_hook.py" or .filename == "install-hooks" or .filename == "Makefile" or .filename == "README.md" or .filename == "docs/users-guide.md" or .filename == "docs/developers-guide.md" then .patch else null end)
}'Length of output: 79982
Verified items:
Remove the unrelated The sandbox could not run the repository validation commands. Use the successful 🧠 Learnings usedYou are interacting with an AI system. |
Untrack the agent tool-output spool; the directory is already ignored.
|
@coderabbitai Have the following now been resolved?
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '== Pull request state and changed .vtcode paths =='
gh pr view 50 --repo leynos/agent-helper-scripts \
--json state,headRefName,baseRefName,statusCheckRollup,files \
--jq '{
state,
headRefName,
baseRefName,
vtcodeFiles: [.files[] | select(.path | startswith(".vtcode/")) | .path],
checks: [.statusCheckRollup[] | {name, status, conclusion}]
}'
printf '%s\n' '== Checked-out branch: .vtcode diff against merge base =='
base="$(git merge-base HEAD origin/main 2>/dev/null || git merge-base HEAD main)"
git diff --name-status "$base"...HEAD -- .vtcode || true
printf '%s\n' '== Checked-out branch: relevant tracked or untracked files =='
git ls-files --cached --others --exclude-standard -- .vtcode \
| rg -n '(^\.vtcode/context/tool_outputs/|^\.vtcode/tasks/current_task\.md$|^\.vtcode/tool-policy\.json$)' \
|| true
printf '%s\n' '== Working-tree changes in .vtcode =='
git status --short -- .vtcode || trueLength of output: 1872
PR The branch also has no
You are interacting with an AI system. |
Closes #37 and #38. Addresses #36 and #40. Issues #39 and #41 are deferred in full.
Survivor-triage PR for the mutation-testing run recorded against
main(b850e2f). It adds targeted unit tests for the highest-value surviving mutants inhooks/post-turn-quality-stop-hook.py, and eliminates or suppresses the equivalent mutants identified at triage.Before/after (full
mutmut run, mutmut 3.6.0, CPython 3.14)Six equivalent mutants are no longer generated (pragma suppression and the removal of a redundant argument — see below). The 129 remaining survivors break down as 117 deferred real gaps (71
prepare_run_stop_checks+ 10run_stop_checks→ #39; 18repo_root+ 18get_make_targets→ #36) and 12 newly identified equivalents (7format_reason, 3parse_make_targets, 2truncate— justified below).Review walkthrough
hooks/test_post_turn_quality_stop_hook.py— new test classes fortruncate(boundary and exact head/tail split),parse_make_targetsandis_missing_makefile(representativemake -qpfixture),detect_categoriesanddefault_categories(full-dict equality across every extension group),format_reason(exact multi-line rendering, the 60-file elision boundary, missing-key command entries), the fullparse_envtuple andparse_max_outputfallback, andrun()(real subprocess capturing text output, theNotADirectoryErrorbranch, the filename-lessFileNotFoundErrorguard). Existing plumbing tests gain exact argv (call_args_list) assertions, exact error-message equality in place of substring checks, wiring assertions incompush_check, and the ahead-by-exactly-one boundary case.hooks/post-turn-quality-stop-hook.py— equivalence handling only; no behavioural change (the full suite and a fresh mutation run confirm).Equivalent mutants (category 2)
mutmut 3.6 honours
# pragma: no mutateonly at statement level (its pragma visitor ignores trailing comments inside call argument lists), so the worklist's four equivalents were handled structurally:parse_envflag defaults (""→"XXXX"onPOST_TURN_ALWAYS_FETCH/POST_TURN_COMPUSH): hoisted into a singleflag_default = "" # pragma: no mutatestatement with a justification comment — any mutated default is still non-truthy toparse_bool_env.run()check=False→check=None/dropped: the argument was the subprocess default, so both mutants were equivalent by construction; the redundant argument is removed and a comment records that non-zero return codes are deliberately surfaced viaCompletedProcess.returncode.POST_TURN_MAX_OUTPUT_CHARSdefault ("12000"→ non-numeric) falls back toparse_max_output's own 12000 default — hoisted and suppressed the same way.Twelve further equivalents remain in the survivor list unsuppressed, because each sits on a line that also carries killable mutants (line-level suppression would shield them):
format_reason(7):c.get("exit_code", …)defaults are unreachable — the failures filter guarantees the key is present;c.get("stdout"/"stderr", None/dropped)defaults are filtered identically to""by theif xcomprehension guard.parse_make_targets(3): corrupting one element of thestartswith(("#", "\t", " "))guard is unobservable — the rule regex rejects those lines anyway.truncate(2):<=→<at both boundaries converges on the same output through the fall-through slice paths.Red-green evidence
Each new test was verified against a hand-applied mutant diff (fail) and the reverted source (pass); cycles were run for
format_reason,run,default_categories,has_unpushed_commits, andparse_env. Representative transcript:(Two same-length mutations initially produced misleading transcripts because Python's mtime-plus-size
.pycinvalidation served stale bytecode; the cycles were re-run withPYTHONDONTWRITEBYTECODE=1and clean caches, and both then behaved correctly.)Deferred work
The 400-line test-budget tolerance for this PR is spent (392 net new test lines). Deferred explicitly:
repo_root(18) andget_make_targets(18) argv/message survivors.prepare_run_stop_checks,run_stop_checks).targets_for_categories,dedup_preserve_order,parse_hook_input,resolve_start_cwd,block_and_print, andfail_stateremain in the no-tests bucket.Validation
make ci(check-fmt, lint, typecheck, full pytest suite — 149 tests) green.mutmut runre-runs after each commit; scopedmutmut run 'hooks.post-turn-quality-stop-hook.x_format_reason*'confirmed the final residual kill.Summary by Sourcery
Move the post-turn quality stop hook out of this repository and update project configuration, documentation, and tests to reflect its removal.
Enhancements:
Build:
CI:
Documentation:
Tests: