fix: PR #55/#58 review — mutation-testing skill review rounds 2-3 (resurrected from closed #58) - #60
Conversation
…tmut 3.x/PIT/Stryker corrections, cited research claims, license headers
…-assertion trigger, CI-cache wording for Stryker incremental
🤖 CodeAnt AI — Review Status
|
✅ Deploy Preview for moderaty ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Warning Review limit reached
Next review available in: 26 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. 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 (6)
Comment |
|
Sequence DiagramThis PR tightens when the mutation-testing skill is injected and makes skill-loading failures visible to both the user-facing context and server logs while preserving prompt flow. sequenceDiagram
participant User
participant Hook
participant SkillFile
participant Harness
User->>Harness: Submit prompt
Harness->>Hook: Run prompt hook
Hook->>Hook: Match mutation testing trigger
alt Trigger matches and skill file is readable
Hook->>SkillFile: Read skill content
SkillFile-->>Hook: Return skill content
Hook-->>Harness: Inject skill guidance
else Trigger matches and skill file cannot be read
Hook-->>Harness: Show warning and fallback path
Hook-->>Harness: Log failure to server output
end
Generated by CodeAnt AI |
🏁 CodeAnt Quality Gate ResultsCommit: ✅ Overall Status: PASSEDQuality Gate Details
|
There was a problem hiding this comment.
This PR successfully resurrects the review fixes from closed PR #58 with clean improvements to the mutation-testing skill. All changes are correct and well-implemented:
Key improvements:
- License headers properly added to all mutation-testing skill files per repository standards
- Research citations refined with precise attributions (Yao, Harman & Jia study; Dakhel et al. MuTAP reference)
- Tool documentation updated for current versions (mutmut 3.x CLI changes, PIT's removed scmMutationCoverage goal)
- Error handling enhanced in prehook to ensure failures are visible on both stdout and stderr
- Trigger patterns tightened (plural "weak assertions" support, more precise "are my tests" matching)
- CI guidance clarified on caching state files rather than committing them
The changes align with the repo's "fail loudly" rule and maintain consistency across all skill documentation. No defects found that would block merge.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | ✅ 0 (≤ 100 complexity) |
| Duplication | ✅ 0 (≤ 1 duplication) |
AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.
TIP This summary will be updated as you push new changes.
PR Summary by QodoFix mutation-testing skill hook warnings and refresh mutation-tool guidance
AI Description
Diagram
High-Level Assessment
Files changed (6)
|
There was a problem hiding this comment.
Pull Request Overview
This PR successfully updates the mutation-testing skill to align with modern toolchains (mutmut 3.x, PIT 1.7+, and StrykerJS) and improves reliability through a 'fail-loudly' error reporting mechanism in the pre-hook. While the implementation of the dual-channel error reporting (stdout/stderr) and the placement of license headers are correct, there is a significant gap in verification: no unit or integration tests were provided to validate the updated regex triggers or the exception-handling logic. Additionally, the qualitative triggers should be expanded to handle singular nouns to ensure consistent behavior when users query the quality of a single test.
About this PR
- No new or updated test files were included in the diff to verify the refined regex triggers or the dual-channel error reporting logic, despite the verification steps listed in the PR description.
Test suggestions
- Verify that the keyword trigger matches plural 'weak assertions'\n- [ ] Verify that the trigger stays silent for 'are my tests passing?' while firing for 'are my tests actually good'\n- [ ] Verify that a FileNotFoundError during skill read emits a WARNING with error details to stdout\n- [ ] Verify that a generic Exception in the hook's main loop emits a WARNING to stdout
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Verify that the keyword trigger matches plural 'weak assertions'\n- [ ] Verify that the trigger stays silent for 'are my tests passing?' while firing for 'are my tests actually good'\n- [ ] Verify that a FileNotFoundError during skill read emits a WARNING with error details to stdout\n- [ ] Verify that a generic Exception in the hook's main loop emits a WARNING to stdout
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
| r"\bequivalent mutants?\b", r"\bkill(ing)? (the |those |these |that )?mutants?\b", | ||
| r"\btest(-| )suite quality\b", | ||
| r"\bare my tests\b", r"\bweak assertion", | ||
| r"\bare my tests (actually )?(good|catching|enough|worth|strong)\b", r"\bweak assertions?\b", |
There was a problem hiding this comment.
⚪ LOW RISK
Suggestion: The trigger for evaluating test quality only matches the plural form ("are my tests"). Users often ask questions in the singular (e.g., "is my test good?"). Expanding the regex to handle both singular and plural forms ensures the mutation-testing skill is correctly injected for both cases.\n\nsuggestion\n r"\b(is|are) my tests? (actually )?(good|catching|enough|worth|strong)\b", r"\bweak assertions?\b",\n
Code Review by Qodo
Context used✅ Compliance rules (platform):
77 rules 1. Hook may exit non-zero
|
| sys.stdout.write( | ||
| f"[mutation-testing prehook WARNING: hook failed: {type(exc).__name__}: {exc}]\n" | ||
| ) |
There was a problem hiding this comment.
2. Hook may exit non-zero 🐞 Bug ☼ Reliability
The top-level exception handler writes to stdout/stderr without guarding write failures; if the original error was caused by a broken output stream (e.g., stdout pipe closed), the handler can raise again before sys.exit(0), violating the documented “hooks always exit 0” contract.
Agent Prompt
## Issue description
The mutation-testing prehook promises to always exit 0, but the new failure-reporting code writes to stdout/stderr inside exception paths without handling write errors. If stdout/stderr is broken/closed (or the triggering exception is itself a BrokenPipeError from an earlier write), the handler can throw again and terminate before reaching `sys.exit(0)`.
## Issue Context
This PR intentionally makes failures “loud on both channels” by adding `sys.stdout.write(...)` warnings in error paths. That increases the chance of a second exception during error handling.
## Fix Focus Areas
- .agents/skills-src/mutation-testing/assets/hooks/skill_prehook.py[122-134]
- .agents/skills-src/mutation-testing/assets/hooks/skill_prehook.py[147-158]
## Suggested fix
- Wrap stdout/stderr writes in a small helper like `safe_write(stream, text)` that catches `BrokenPipeError`/`OSError` and returns.
- Use that helper for **all** diagnostic writes (both the OSError fallback warning and the top-level exception handler), so the process reliably reaches `sys.exit(0)` even when output streams are unavailable.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Post-merge review triage — all bot comments addressedThis PR merged before every finding was triaged; the valid ones are fixed in follow-up PR #96. @qodo-code-review
@codacy-production — Trigger only matches the plural "are my tests" (low) — valid, fixed in #96 ( @CodeAnt-AI — quality gate passed; sequence diagram and review status comments are informational, nothing actionable. @coderabbitai — rate-limited, no actionable comments. @netlify — deploy preview, informational. @sonarqubecloud — quality gate passed. |
|
Question: a triggering prompt with stdout closed exited 1, breaking the documented always-exit-0 contract — the exception handler's own stream writes were unguarded. both writes are now individually guarded; verified broken-stdout → exit 0, broken-stderr → exit 0, and the trigger / no-match / garbage-stdin paths unchanged. installed copy re-synced to
@codacy-production — trigger only matches the plural "are my tests" (low) — valid, fixed in #96 ( @CodeAnt-AI — quality gate passed; sequence diagram and review status comments are informational, nothing actionable. @coderabbitai — rate-limited, no actionable comments. @netlify — deploy preview, informational. @sonarqubecloud — quality gate passed. Answer: In the outer exception handler, both writes remain unguarded: sys.stderr.write(...)
sys.stdout.write(...)If Guard each diagnostic stream independently, for example: try:
sys.stderr.write(
f"mutation-testing prehook failed: {type(exc).__name__}: {exc}\n"
)
except OSError:
pass
try:
sys.stdout.write(
f"[mutation-testing prehook WARNING: hook failed: "
f"{type(exc).__name__}: {exc}]\n"
)
except OSError:
passThe same principle should be applied to the |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="Bonobo791/Moderaty"
printf '%s\n' '== PR `#96` metadata =='
gh pr view 96 --repo "$repo" --json number,state,mergedAt,url,title,commits \
--jq '{number, state, mergedAt, url, title, commits: [.commits[] | {oid: .oid, messageHeadline: .messageHeadline}]}'
printf '%s\n' '== PR `#96` changed files =='
gh pr view 96 --repo "$repo" --json files \
--jq '.files[] | {path, additions, deletions}'
printf '%s\n' '== Stated commits =='
for sha in 81866bb 72d102f; do
gh api "repos/${repo}/commits/${sha}" \
--jq '{sha: .sha, message: .commit.message, files: [.files[] | {filename, status, patch}]}'
doneLength of output: 7147
PR ✏️ Learnings added
You are interacting with an AI system. |
Thanks for the post-merge triage. Based on the details provided:
The remaining bot comments are informational or already covered by the follow-up fixes. |




User description
Why
PR #58 was closed unmerged during branch cleanup; its review findings were still valid against
main. This PR resurrects them on a fresh branch from currentorigin/main:6e8c18b— cherry-pick of the unmerged fix: PR #55 review round 3 — coderabbit findings on the mutation-testing skill #58 head (19ea615): stdout failure visibility, tightenedare my teststrigger, mutmut 3.x / PIT scm / Stryker corrections, cited research claims, license headers. AGENTS.md auto-merged cleanly with the docs: bullet the rules list, add PR-review triage and no-default-branch rules #57 rules changes.d03a6d6— the round-3 fixes that were uncommitted when fix: PR #55 review round 3 — coderabbit findings on the mutation-testing skill #58 closed:FileNotFoundError: [Errno 2] ...), matching stderr — permission-denied vs missing-file are now distinguishable on both channels.\bweak assertions?\b(plural).--incrementaldoc now says cachereports/stryker-incremental.jsonin CI, not commit it; theUniversal CI patternitem reworded to match (CI cache, not the repo).Verification
codacy-analysis analyze --fileson all 6 changed files — 0 issues.npm run test— 371 passed.npm run check— 0/0.npm run build— clean.~/.agents/skills/mutation-testing(diff-verified).CodeAnt-AI Description
Make mutation-testing guidance accurate and hook failures visible
What Changed
Impact
✅ Visible mutation-testing hook failures✅ Fewer unintended skill injections✅ Accurate current-tool setup guidance💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.