Repository navigation
Conversation
…e unchanged _fixpoint_reached is called directly and also through the _fixpoint_without counterfactual used by the schema-fail companion preserve. If a call changed instance state, the counterfactual could alter the decision it is only meant to inspect. The run-level tests now wrap the real _fixpoint_reached for an earned clean round, a companion counterfactual round, a recurring P3 plus coverage round and a failed host attempt. Each requires an unchanged machine snapshot and the same result on a repeated call, and _fixpoint_without must hand back the identical findings list. The run-level P3 density and distinct-per-file thresholds had no run-level negatives. Over-threshold rounds beside incomplete coverage must now reset, and within-threshold rounds must preserve. Another case pins that a schema-invalid reviewer payload whose audit copy cannot be encoded, beside an attempted payload, does not preserve credit; a variant makes the receipts directory read-only. Test-only; src is unchanged.
…eipt writes The receipt-write variant made the receipts directory read-only, which has no effect for root, so the case failed when the suite ran as root. It now patches Path.write_text so that creating a new file directly under the receipts directory raises PermissionError with errno EACCES for that round only. The case runs for any uid and asserts that a write was refused and that the gate error carries the errno. _coverage_round_with_counterfactual now installs the side-effect guard on every _fixpoint_reached call and asserts that both the direct call and the _fixpoint_without call were recorded. The schema-fail companion negatives compare an instance-attribute snapshot before and after the direct call and are guarded through the counterfactual. Their companion finding makes the direct call return early, so these cases do not reach the hold, protocol-failure, falsify-failure or recurring P0/P1 returns; the next commit adds cases for those. The snapshot docstrings now say that the snapshot covers instance attributes only; files, module-level and class-level state are outside it.
Adds one guarded case per return of _fixpoint_reached on a machine that carries only that case's findings: new CONFIRMED, FIXED reversion, confirmation after a protocol failure, hold, falsify failure, UNCERTAIN, CLEAN, recurring P1, recurring P2, P3 distinct and P3 density. Each case asserts the expected result, which pins the return taken, and asserts that the instance-attribute snapshot is unchanged with no guard violation. The schema-fail companion machines used by the previous commit carry a companion finding that exits earlier, so on their own they never reached the hold, protocol, falsify-failed and P0/P1 returns.
…factual The every-return cases and the schema-fail companion negatives compared a snapshot before and after the direct call, which the side-effect guard already records as a violation. The duplicate comparison is dropped. The companion negatives asserted only the direct call site, so nothing showed that the counterfactual call was guarded. They now assert that _fixpoint_without was recorded for every case except the hold case, which the preserve check rejects before it computes the counterfactual.
The six tests added for the side-effect-free fixpoint check and for the run-level P3 density and distinct-per-file negatives carried tracking labels in their names, section headers, comments and docstrings. Those labels tell a reader of the tests nothing about what is checked, and they go stale once the work they pointed at is closed. Rename the tests after the behavior they check and reword the comments and docstrings to describe that behavior only. Test bodies, parameters and assertions are unchanged, so the collected cases are the same apart from the new names.
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR only adds tests. Nothing under
src/changes:git diff 22d9fa98 -- srcis empty, and an AST comparison of all 142.pyfiles undersrc/at 22d9fa9 and at this tip shows they are identical. The two touched files aretests/test_review_interruption_source_controls.pyandtests/test_phase6_schema_coverage_credit.py(+444 lines, no deletions).Why
_fixpoint_reachedgets called directly, and also through the_fixpoint_withoutcounterfactual that the schema-fail companion preserve uses. If either call changed instance state, the counterfactual could change the very decision it is only supposed to inspect. Until now no test checked for that. The run-level P3 density and distinct-per-file thresholds also had no run-level negatives.What the tests cover
_fixpoint_reachedin four run-level scenarios: an earned clean round, a companion counterfactual round, a recurring P3 with a coverage round, and a failed host attempt. Each one requires an unchanged instance-attribute snapshot and the same result on a repeated call._fixpoint_withoutalso has to hand back the identical findings list object._fixpoint_reachedgets its own direct case, run on a machine that holds only that case's findings. The returns are new CONFIRMED, FIXED reversion, confirmation after a protocol failure, hold, falsify failure, UNCERTAIN, CLEAN, recurring P1, recurring P2, P3 distinct and P3 density. The expected result pins which return is taken, and the guard must record no violation._fixpoint_without. The hold case doesn't, because the preserve check rejects it before computing the counterfactual, and the test pins that as well._p3_over_thresholdunit negatives.PermissionError(EACCES) for that round by patchingPath.write_text. It doesn't rely on directory permissions, so it also works when the suite runs as root.The snapshot covers instance attributes only. Files, module-level state and class-level state are not in it, and the docstrings say so.
How this was checked
Path.write_textpatch._fixpoint_reachedappended toinfra_errorsjust before that return. Before this branch, 9 of those 10 mutants survived the two affected test files. With the every-return cases added, all 10 are killed. That was measured before the last two commits. The first of those only touches the companion test (+7/-4), and after it I re-ran three mutants: distinct-threshold (3 failures), P2 (2) and aninfra_errorsappend (42). All three are still killed. The last commit only renames six tests to describe what they check; their bodies, assertions and parametrization are unchanged.ruff checkandruff format --checkare clean on the changed files.