scriptoriumadmin/strix
1
0
Fork 0
mirror of https://github.com/usestrix/strix.git synced 2026-10-01 02:03:55 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

Make fix handoffs actionable and require customer unit tests

Browse source
This commit is contained in:
Jonathan Singer 2026-09-29 13:27:27 -04:00
parent f03fd72d79
commit 0faa7b7da1
4 changed files with 178 additions and 12 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
strix/fix/__init__.py
View file

@ -20,6 +20,7 @@ from strix.fix.contracts import (
RepairStatus,
RepositoryTestPlan,
ReproductionSpec,
ReviewConcern,
SourceIdentity,
SourceIdentityKind,
VerificationDecision,
@ -61,6 +62,7 @@ __all__ = [
"RepairStatus",
"RepositoryTestPlan",
"ReproductionSpec",
"ReviewConcern",
"SourceIdentity",
"SourceIdentityKind",
"VerificationDecision",

7
strix/fix/contracts.py
View file

@ -291,6 +291,11 @@ class VerificationHarness(ContractModel):
content: str
class ReviewConcern(ContractModel):
kind: Literal["repair_needed", "customer_prerequisite", "optional_follow_up"]
summary: str = Field(min_length=1)
class VerifierResult(ContractModel):
decision: VerificationDecision
summary: str
@ -309,6 +314,8 @@ class VerifierResult(ContractModel):
review_basis: Literal["execution", "code_review"] | None = None
regression_test_valid: bool = False
unit_test_coverage_valid: bool = False
# None preserves historical reviews; new reviewers classify every concern.
concerns: list[ReviewConcern] | None = None
class RepairOutcome(ContractModel):

84
strix/fix/prepare.py
View file

@ -421,6 +421,10 @@ def _verification_passes(checks: list[CheckResult], verifier: VerifierResult) ->
and any(
item.purpose == "regression" and item.status is CheckStatus.PASSED for item in checks
)
and any(
item.purpose == "unit" and item.required and item.status is CheckStatus.PASSED
for item in checks
)
and len({item.environment_id for item in checks if item.environment_id}) <= 1
and len({item.source_digest for item in checks if item.source_digest}) <= 1
and verifier.decision is VerificationDecision.VERIFIED
@ -439,7 +443,49 @@ def _test_plan_gaps(repair: RepairOutcome, manifest: list[FileManifestEntry]) ->
return ["The repair must supply a regression test and the repository unit-test commands."]
changed = {entry.path for entry in manifest if entry.operation != "delete"}
missing = [path for path in plan.regression_files if path not in changed]
return [f"Regression test must be added or updated in the patch: {path}" for path in missing]
gaps = [f"Regression test must be added or updated in the patch: {path}" for path in missing]
if not plan.unit_tests:
gaps.append("Supply the customer's existing unit-test suite for the affected code.")
if plan.no_unit_tests_reason:
gaps.append(plan.no_unit_tests_reason)
return gaps
def _resolve_review(verifier: VerifierResult) -> VerifierResult:
"""Derive the outcome from actionable concerns, retaining legacy record support."""
if verifier.concerns is None:
return verifier
repairs = [c.summary for c in verifier.concerns if c.kind == "repair_needed"]
prerequisites = [c.summary for c in verifier.concerns if c.kind == "customer_prerequisite"]
notes = [c.summary for c in verifier.concerns if c.kind == "optional_follow_up"]
approved = (
verifier.security_invariant_closed
and verifier.regression_test_valid
and verifier.unit_test_coverage_valid
)
decision = (
VerificationDecision.REJECTED
if repairs
else VerificationDecision.INCONCLUSIVE
if prerequisites or not approved
else VerificationDecision.VERIFIED
)
return verifier.model_copy(
update={
"decision": decision,
"repairable": bool(repairs),
"gaps": [*repairs, *prerequisites],
"notes": list(dict.fromkeys([*verifier.notes, *notes])),
"blocker": PreparationBlocker(
kind=BlockerKind.EXTERNAL_CONFIGURATION,
summary="A required customer prerequisite is missing.",
user_action="\n".join(prerequisites),
details=prerequisites,
)
if prerequisites
else None,
}
)
def _test_commands(repair: RepairOutcome) -> list[CommandSpec]:
@ -562,6 +608,10 @@ async def prepare_fix( # noqa: PLR0915
checks = record.checks
record.workspace_digest = await _workspace_digest(workspace)
manifest, _, _ = await build_git_manifest(workspace)
can_retry = (
attempt < resolved_policy.max_repair_attempts
and record.repair.status is not RepairStatus.BLOCKED
)
if not manifest:
if record.repair.blocker:
return await finish(
@ -570,14 +620,18 @@ async def prepare_fix( # noqa: PLR0915
blocker=record.repair.blocker,
gaps=record.repair.gaps,
)
record.repair.gaps.append(
"No changed files were found. Use the file tools to implement the fix and "
"regression test, then submit their validation commands."
)
if can_retry:
continue
return await finish(
PreparationState.FAILED, "The repair did not change repository source."
PreparationState.FAILED,
"The repair did not change repository source.",
gaps=record.repair.gaps,
)
await manifest_builder(workspace) # Save the patch before any test can fail.
can_retry = (
attempt < resolved_policy.max_repair_attempts
and record.repair.status is not RepairStatus.BLOCKED
)
plan_gaps = _test_plan_gaps(record.repair, manifest)
if plan_gaps:
record.repair.gaps = list(dict.fromkeys([*record.repair.gaps, *plan_gaps]))
@ -585,7 +639,7 @@ async def prepare_fix( # noqa: PLR0915
continue
return await finish(
PreparationState.BLOCKED,
"The patch is saved, but its regression-test handoff is incomplete.",
"The patch is saved, but its required test handoff is incomplete.",
blocker=record.repair.blocker,
gaps=record.repair.gaps,
)
@ -625,7 +679,7 @@ async def prepare_fix( # noqa: PLR0915
gaps=[f"{item.name}: {item.status}" for item in failed],
)
# Review the exact repair and its native test results. No second harness is required.
verifier = await verify(context, checks)
verifier = _resolve_review(await verify(context, checks))
record.verifier = verifier
if record.workspace_digest != await _workspace_digest(workspace):
return await finish(
@ -640,6 +694,13 @@ async def prepare_fix( # noqa: PLR0915
"The independent reviewer found a repair or regression-test defect.",
gaps=verifier.gaps,
)
if verifier.blocker:
return await finish(
PreparationState.BLOCKED,
verifier.blocker.summary,
blocker=verifier.blocker,
gaps=verifier.gaps,
)
if record.repair.status is RepairStatus.BLOCKED:
return await finish(
PreparationState.BLOCKED,
@ -647,12 +708,15 @@ async def prepare_fix( # noqa: PLR0915
blocker=record.repair.blocker,
gaps=record.repair.gaps,
)
if _verification_passes(checks, verifier) and not record.repair.gaps:
# Typed review explicitly resolves the repair agent's observations. Historical
# free-text gaps remain blocking until a reviewer classifies them.
repair_gaps = record.repair.gaps if verifier.concerns is None else []
if _verification_passes(checks, verifier) and not repair_gaps:
return await finish(
PreparationState.READY,
"Tests and required checks passed; independent review approved a draft PR.",
)
gaps = list(dict.fromkeys([*record.repair.gaps, *verifier.gaps]))
gaps = list(dict.fromkeys([*repair_gaps, *verifier.gaps]))
if not gaps:
gaps = ["Independent review could not confirm the repair and native-test coverage."]
return await finish(

97
tests/test_fix_preparation.py
View file

@ -25,6 +25,7 @@ from strix.fix.contracts import (
RepairStatus,
RepositoryTestPlan,
ReproductionSpec,
ReviewConcern,
SourceIdentity,
SourceIdentityKind,
VerificationDecision,
@ -93,7 +94,11 @@ def _workspace(tmp_path: Path) -> tuple[Path, str]:
_git(workspace, "config", "user.email", "test@example.com")
_git(workspace, "config", "user.name", "Test")
(workspace / "app.py").write_text("def result():\n return 'unsafe'\n", encoding="utf-8")
_git(workspace, "add", "app.py")
(workspace / "test_existing.py").write_text(
"import unittest\nfrom app import result\nclass Existing(unittest.TestCase):\n"
" def test_result_type(self): self.assertIsInstance(result(), str)\n"
)
_git(workspace, "add", "app.py", "test_existing.py")
_git(workspace, "commit", "-m", "initial")
return workspace, _git(workspace, "rev-parse", "HEAD")
@ -179,7 +184,11 @@ async def _noop_repair(
regression_test=CommandSpec(
name="regression", argv=[sys.executable, "-m", "unittest", "test_app"]
),
no_unit_tests_reason="The fixture has no prior unit suite.",
unit_tests=[
CommandSpec(
name="existing suite", argv=[sys.executable, "-m", "unittest", "test_existing"]
)
],
),
status=RepairStatus.COMPLETE,
summary="The repository fix is ready for independent evaluation.",
@ -586,6 +595,90 @@ async def test_exhausted_repair_can_be_validated_from_saved_work(tmp_path: Path)
assert result.state is PreparationState.READY
@pytest.mark.asyncio
@pytest.mark.parametrize("recover", [True, False])
async def test_empty_completion_gets_actionable_bounded_retry(
tmp_path: Path, recover: bool
) -> None:
workspace, commit = _workspace(tmp_path)
async def repair(context, checks):
if context.attempt == 2:
assert "No changed files" in context.feedback[0].repair.gaps[0]
if recover:
return await _noop_repair(context, checks)
return RepairOutcome(status=RepairStatus.COMPLETE, summary="Claimed completion")
result = await prepare_fix(
_request(_candidate(commit)), workspace, repair=repair, verify=_verified
)
assert result.attempts == 2
assert result.state is (PreparationState.READY if recover else PreparationState.FAILED)
if not recover:
assert not result.final_file_manifest
assert "No changed files" in result.gaps[0]
@pytest.mark.asyncio
async def test_missing_customer_suite_cannot_be_waived_with_an_explanation(tmp_path: Path) -> None:
workspace, commit = _workspace(tmp_path)
async def repair(context, checks):
outcome = await _noop_repair(context, checks)
outcome.test_plan.unit_tests = []
outcome.test_plan.no_unit_tests_reason = "No existing suite was found."
return outcome
async def review(*_args):
pytest.fail("A reviewer cannot waive required customer unit tests")
result = await prepare_fix(
_request(_candidate(commit)), workspace, repair=repair, verify=review
)
assert result.state is PreparationState.BLOCKED
assert result.final_file_manifest
assert any("existing unit-test suite" in gap for gap in result.gaps)
@pytest.mark.asyncio
@pytest.mark.parametrize("kind", ["repair_needed", "customer_prerequisite", "optional_follow_up"])
async def test_controller_derives_review_action_and_resolves_repair_observations(
tmp_path: Path, kind: str
) -> None:
workspace, commit = _workspace(tmp_path)
async def repair(context, checks):
outcome = await _noop_repair(context, checks)
outcome.gaps = ["An additional deployment check could help."]
if context.attempt == 2:
assert context.feedback[0].verifier.gaps == ["Inspect the companion configuration."]
return outcome
async def review(context, checks):
result = await _verified(context, checks)
# The controller must derive the outcome even if the proposed decision says verified.
result.concerns = (
[]
if context.attempt == 2
else [ReviewConcern(kind=kind, summary="Inspect the companion configuration.")]
)
return result
result = await prepare_fix(
_request(_candidate(commit)), workspace, repair=repair, verify=review
)
assert result.attempts == (2 if kind == "repair_needed" else 1)
if kind == "customer_prerequisite":
assert result.state is PreparationState.BLOCKED
assert result.blocker.user_action == "Inspect the companion configuration."
else:
assert result.state is PreparationState.READY
assert not result.gaps
assert not result.verifier.gaps
if kind == "optional_follow_up":
assert result.verifier.notes == ["Inspect the companion configuration."]
@pytest.mark.asyncio
@pytest.mark.parametrize(
"defect",