Skip to content

Commit e0cb0ae

Browse files
committed
Added information about script timeouts and added proper context when failing a previous test
1 parent dcce796 commit e0cb0ae

11 files changed

Lines changed: 550 additions & 62 deletions

‎git_utils.py‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,10 @@
1313
CONFORMANCE_TESTS_PASSED_COMMIT_MESSAGE = (
1414
"[Codeplain] Fixed issues in the implementation code identified during conformance testing"
1515
)
16+
# Committed at the moment the integrity reviewer approves a conformance fix — the
17+
# approval advances the review anchor, so the approved changes are committed (to the
18+
# build repo and every conformance-test repo the fix touched) in the same event.
19+
APPROVED_CONFORMANCE_FIX_COMMIT_MESSAGE = "[Codeplain] Approved conformance fix (tests of functionality {} in {})"
1620

1721
# Following messages are used as checkpoints in the git history
1822
# Changing them will break backwards compatibility so change them with care

‎render_machine/actions/agent_fix_conformance_test.py‎

Lines changed: 146 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,12 @@
66
from memory_management import MemoryManager
77
from plain2code_console import console
88
from plain2code_trace import preview, trace
9-
from render_machine import fix_signals
9+
from render_machine import fix_signals, render_utils
1010
from render_machine.actions.base_action import BaseAction
1111
from render_machine.agent import agent_runner
1212
from render_machine.agent.tool_executor import ToolExecutor
1313
from render_machine.agent.tools import (
14+
build_sandbox_contract,
1415
delete_file,
1516
edit_file,
1617
get_session_changes,
@@ -88,18 +89,24 @@ def execute(self, render_context: RenderContext, previous_action_payload: Any |
8889
)
8990

9091
# The file change tracker is owned exclusively by ReviewConformanceFixAction:
91-
# it is cleared there once a fix cycle is reviewed (approved or reverted on
92-
# rejection). The tracker must NOT be reset here, because a fix can loop back
93-
# into fixing several times (e.g. when its changes still fail the tests) before
94-
# the tests pass and a review actually runs. Resetting here would drop the
95-
# original-file snapshots captured by earlier attempts, leaving the reviewer
96-
# with only the last attempt's diff instead of the cumulative diff since the
97-
# pre-fix baseline.
92+
# it is cleared there when a fix is APPROVED (and committed), and reverted
93+
# only when repeated rejections reset the tree to the last approved state. A
94+
# plain rejection clears nothing — the rejected changes stay in the working
95+
# tree for this agent to correct, and the tracker keeps accumulating so the
96+
# next review sees the cumulative diff since the last approved state. The
97+
# tracker must NOT be reset here, because a fix can loop back into fixing
98+
# several times (failed tests, rejections) before a review approves; resetting
99+
# would drop the original-file snapshots captured by earlier attempts.
98100
conformance_test_folder = self._get_conformance_test_folder(render_context)
99101

100102
# Build context
101103
specifications = self._build_specifications_text(render_context)
102104
acceptance_tests = self._build_acceptance_tests_text(render_context)
105+
# During regression the failing test belongs to a different FRID (often a
106+
# different module) than the one being implemented — its own specification
107+
# must be provided explicitly, or the agent fixes a test whose defining spec
108+
# it has never seen.
109+
failing_test_specifications = self._build_failing_test_specifications_text(render_context)
103110

104111
# Diffs of what this render step just produced. The conformance tests were
105112
# passing before this functionality was implemented, so these diffs point the
@@ -159,6 +166,7 @@ def execute(self, render_context: RenderContext, previous_action_payload: Any |
159166
linked_resource_paths=linked_resource_paths,
160167
acceptance_tests=acceptance_tests,
161168
conformance_test_folder=conformance_test_folder,
169+
failing_test_specifications=failing_test_specifications,
162170
)
163171
self._add_optional_task_params(
164172
task_params,
@@ -226,6 +234,7 @@ def execute(self, render_context: RenderContext, previous_action_payload: Any |
226234
linked_resource_paths=linked_resource_paths,
227235
acceptance_tests=acceptance_tests,
228236
conformance_test_folder=conformance_test_folder,
237+
failing_test_specifications=failing_test_specifications,
229238
)
230239
self._add_optional_task_params(
231240
task_params,
@@ -321,6 +330,7 @@ def execute(self, render_context: RenderContext, previous_action_payload: Any |
321330
"specifications": specifications,
322331
"acceptance_tests": acceptance_tests,
323332
"conformance_test_folder": conformance_test_folder,
333+
"failing_test_specifications": failing_test_specifications,
324334
}
325335

326336
return self.FIX_APPLIED, {
@@ -453,7 +463,13 @@ def _record_previous_attempt_outcome(ctx, review_rejection_feedback: str, prepar
453463
if last_attempt.get("outcome"):
454464
return
455465
if review_rejection_feedback:
456-
last_attempt["outcome"] = "rejected by the integrity reviewer"
466+
# Carry the reviewer's reasons onto the ledger: fresh sessions receive the
467+
# ledger (not the feedback payload), and "rejected" without the why is
468+
# exactly what lets a successor re-attempt the same violation.
469+
snippet = " ".join(review_rejection_feedback.split())
470+
if len(snippet) > 600:
471+
snippet = snippet[:600] + "…"
472+
last_attempt["outcome"] = f"rejected by the integrity reviewer: {snippet}"
457473
elif prepare_environment_failed:
458474
last_attempt["outcome"] = "build/compile failed after the fix; the tests never ran"
459475
elif ctx.failure_signature_streak >= 1:
@@ -559,6 +575,7 @@ def _build_start_task_params(
559575
linked_resource_paths: list[str],
560576
acceptance_tests: str,
561577
conformance_test_folder: str,
578+
failing_test_specifications: str = "",
562579
) -> dict:
563580
"""Build the task params used to start a fresh fix-agent session.
564581
@@ -583,7 +600,11 @@ def _build_start_task_params(
583600
"memory_folder": render_context.memory_manager.memory_folder,
584601
"memory_file_names": MemoryManager.list_memory_files(render_context.memory_manager.memory_folder),
585602
"keep_session_alive": True, # Mark this as a persistent session
603+
"test_script_timeout_seconds": render_utils.effective_test_script_timeout(render_context),
604+
"sandbox_contract": build_sandbox_contract(render_context),
586605
}
606+
if failing_test_specifications:
607+
task_params["failing_test_specifications"] = failing_test_specifications
587608

588609
# Orientation seeds: codebase map (boosted by spec terms and the failing test
589610
# output, so the files implicated in the failure keep their outlines when the
@@ -718,6 +739,103 @@ def _build_acceptance_tests_text(self, render_context: RenderContext) -> str:
718739
return ""
719740
return "\n".join(acceptance_tests)
720741

742+
def _build_failing_test_specifications_text(self, render_context: RenderContext) -> str:
743+
"""Build the failing test's own specification when it differs from the implementing FRID.
744+
745+
The main Specification section is centered on the FRID being implemented.
746+
During regression the failing test verifies a different FRID — often one of
747+
a required module, which appears there only as one-line functionality
748+
summaries. Without this section the agent (and the reviewer) judge a test
749+
whose defining spec they have never seen.
750+
"""
751+
ctx = render_context.conformance_tests_running_context
752+
if ctx is None or ctx.current_testing_frid is None:
753+
return ""
754+
if (
755+
ctx.current_testing_module_name == render_context.module_name
756+
and ctx.current_testing_frid == render_context.frid_context.frid
757+
):
758+
# The failing test IS the implementing FRID's — the main Specification
759+
# already centers on it.
760+
return ""
761+
762+
parts = [
763+
f"Failing test: module `{ctx.current_testing_module_name}`, "
764+
f"functionality (FRID) {ctx.current_testing_frid}."
765+
]
766+
767+
if ctx.current_testing_module_name == render_context.module_name:
768+
# Same module, earlier FRID: the module's definitions and requirements are
769+
# already in the main Specification — add the failing functionality's text.
770+
failing_fr_text = self._get_failing_functional_requirement_text(ctx)
771+
if not failing_fr_text:
772+
return ""
773+
parts.append(f"### Functional requirement under test\n{failing_fr_text}")
774+
return "\n\n".join(parts)
775+
776+
# Cross-module: pull the failing module's own spec sections from its plain
777+
# source (available on the requires chain), falling back to the functional
778+
# requirement text stored in its conformance_tests.json.
779+
specifications = self._get_failing_module_specifications(render_context, ctx)
780+
if specifications:
781+
if specifications.get(plain_spec.DEFINITIONS):
782+
parts.append(f"### Definitions\n{chr(10).join(specifications[plain_spec.DEFINITIONS])}")
783+
if specifications.get(plain_spec.NON_FUNCTIONAL_REQUIREMENTS):
784+
parts.append(
785+
f"### Non-Functional Requirements\n"
786+
f"{chr(10).join(specifications[plain_spec.NON_FUNCTIONAL_REQUIREMENTS])}"
787+
)
788+
if specifications.get(plain_spec.TEST_REQUIREMENTS):
789+
parts.append(f"### Test Requirements\n{chr(10).join(specifications[plain_spec.TEST_REQUIREMENTS])}")
790+
functional_requirements = specifications.get(plain_spec.FUNCTIONAL_REQUIREMENTS) or []
791+
if functional_requirements:
792+
parts.append(f"### Functional requirement under test\n{functional_requirements[-1]}")
793+
else:
794+
failing_fr_text = self._get_failing_functional_requirement_text(ctx)
795+
if failing_fr_text:
796+
parts.append(f"### Functional requirement under test\n{failing_fr_text}")
797+
798+
return "\n\n".join(parts) if len(parts) > 1 else ""
799+
800+
@staticmethod
801+
def _get_failing_module_specifications(render_context: RenderContext, ctx) -> dict | None:
802+
"""Extract the failing module's spec sections for the failing FRID, if resolvable."""
803+
plain_module = getattr(render_context, "plain_module", None)
804+
candidates = []
805+
if plain_module is not None:
806+
candidates = plain_module.all_required_modules
807+
elif render_context.required_modules:
808+
candidates = render_context.required_modules
809+
failing_module = next(
810+
(module for module in candidates if module.module_name == ctx.current_testing_module_name), None
811+
)
812+
if failing_module is None:
813+
return None
814+
try:
815+
specifications, _ = plain_spec.get_specifications_for_frid(
816+
failing_module.plain_source, ctx.current_testing_frid
817+
)
818+
return specifications
819+
except Exception as e:
820+
console.warning(f"Could not extract the failing module's specifications for fixing context: {e}")
821+
return None
822+
823+
@staticmethod
824+
def _get_failing_functional_requirement_text(ctx) -> str:
825+
"""The failing FRID's functional requirement text, from the loaded test specs.
826+
827+
``current_testing_frid_specifications`` is a spec dict for same-module tests
828+
and the functional requirement string (from conformance_tests.json) for
829+
cross-module tests.
830+
"""
831+
specs = ctx.current_testing_frid_specifications
832+
if isinstance(specs, dict):
833+
functional_requirements = specs.get(plain_spec.FUNCTIONAL_REQUIREMENTS) or []
834+
return functional_requirements[-1] if functional_requirements else ""
835+
if isinstance(specs, str):
836+
return specs
837+
return ""
838+
721839
def _get_linked_resource_paths(self, render_context: RenderContext) -> list[str]:
722840
"""Get list of linked resource paths (not content) for the agent to read if needed."""
723841
linked_resources = render_context.frid_context.linked_resources
@@ -736,12 +854,29 @@ def _build_continuation_message(
736854
"""Build a message to continue the agent session with new test results and feedback."""
737855
parts = []
738856

739-
# Review feedback
857+
# Review feedback. The review only runs after a fix makes the tests pass, so
858+
# on this path the tests currently PASS with the (kept) changes — the failure
859+
# sections below do not apply.
740860
if review_rejection_feedback:
741861
parts.append(
742862
f"The reviewer examined your fix and evaluated it according to the engineering integrity guidelines. The reviewer rejected your fix with this reviewer feedback:\n\n{review_rejection_feedback}\n"
743863
)
744-
parts.append("Please address the reviewer's concerns and try again.\n")
864+
parts.append(
865+
"Your changes were NOT reverted — they are still in the working tree, and the conformance "
866+
"tests currently pass with them. Correct the changes in place: rework or remove the parts "
867+
"the reviewer flagged, keep any parts the reviewer explicitly endorsed, and then submit the "
868+
"corrected fix.\n"
869+
)
870+
parts.append("\nNext steps:\n")
871+
parts.append("1. Thoroughly read the reviewer feedback above.\n")
872+
parts.append(
873+
"2. Re-read your cumulative changes (use get_session_changes) and identify which of them the "
874+
"reviewer flagged.\n"
875+
)
876+
parts.append("3. Correct the flagged changes in place so the fix addresses the root cause legitimately.\n")
877+
parts.append("4. Verify the corrected code by reading the edited files.\n")
878+
parts.append("5. When you are finished, submit the corrected fix using the submit_fix tool.\n")
879+
return "\n".join(parts)
745880

746881
if prepare_environment_failed:
747882
# The build/compile step failed after the fix, so the conformance tests never
@@ -781,8 +916,6 @@ def _build_continuation_message(
781916
parts.append(f" - The environment preparation output file at: {prepare_environment_output_file}\n")
782917
if not prepare_environment_failed and test_output_file:
783918
parts.append(f" - The test output file at: {test_output_file}\n")
784-
if review_rejection_feedback:
785-
parts.append(" - The reviewer feedback\n")
786919
parts.append(
787920
"2. Locate and read the relevant test code and the implementation code that is being tested. Make sure you understand the root cause of the failure.\n"
788921
)

‎render_machine/actions/agent_fix_unit_tests.py‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,12 @@
55
import plain_spec
66
import repo_map
77
from memory_management import MemoryManager
8+
from render_machine import render_utils
89
from render_machine.actions.base_action import BaseAction
910
from render_machine.agent import agent_runner
1011
from render_machine.agent.tool_executor import ToolExecutor
1112
from render_machine.agent.tools import (
13+
build_sandbox_contract,
1214
delete_file,
1315
edit_file,
1416
grep,
@@ -57,6 +59,8 @@ def execute(self, render_context: RenderContext, previous_action_payload: Any |
5759
"module_name": render_context.module_name,
5860
"memory_folder": render_context.memory_manager.memory_folder,
5961
"memory_file_names": MemoryManager.list_memory_files(render_context.memory_manager.memory_folder),
62+
"test_script_timeout_seconds": render_utils.effective_test_script_timeout(render_context),
63+
"sandbox_contract": build_sandbox_contract(render_context),
6064
}
6165
# Orientation seeds: codebase map (boosted by spec terms and the failing test
6266
# output, so implicated files keep their outlines when the map is over budget)

‎render_machine/actions/agent_render_conformance_tests.py‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,12 @@
66
import repo_map
77
from memory_management import MemoryManager
88
from plain2code_console import console
9+
from render_machine import render_utils
910
from render_machine.actions.base_action import BaseAction
1011
from render_machine.agent import agent_runner
1112
from render_machine.agent.tool_executor import ToolExecutor
1213
from render_machine.agent.tools import (
14+
build_sandbox_contract,
1315
delete_file,
1416
edit_file,
1517
grep,
@@ -142,7 +144,12 @@ def _render_conformance_tests(self, render_context: RenderContext):
142144
def _add_orientation_params(
143145
task_params: dict, render_context: RenderContext, conformance_tests_folder_name: str, specifications: str
144146
) -> None:
145-
"""Seed the session with the codebase map and the module's implementation history."""
147+
"""Seed the session with orientation context shared by both render paths.
148+
149+
Codebase map, the module's implementation history, the harness time budget
150+
(tests must be authored to fit it — never by bending spec-mandated values),
151+
and the file-access sandbox contract.
152+
"""
146153
repo_map_text = repo_map.build_repo_map_param(
147154
render_context,
148155
conformance_tests_folder=conformance_tests_folder_name,
@@ -153,6 +160,8 @@ def _add_orientation_params(
153160
code_brief = repo_map.read_code_brief(render_context.build_folder)
154161
if code_brief:
155162
task_params["code_brief"] = code_brief
163+
task_params["test_script_timeout_seconds"] = render_utils.effective_test_script_timeout(render_context)
164+
task_params["sandbox_contract"] = build_sandbox_contract(render_context)
156165

157166
def _render_acceptance_test(self, render_context: RenderContext):
158167
if plain_spec.ACCEPTANCE_TESTS not in render_context.frid_context.specifications:

‎render_machine/actions/agent_render_functional_requirement.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
from render_machine.agent import agent_runner
1111
from render_machine.agent.tool_executor import ToolExecutor
1212
from render_machine.agent.tools import (
13+
build_sandbox_contract,
1314
delete_file,
1415
edit_file,
1516
grep,
@@ -59,6 +60,8 @@ def execute(self, render_context: RenderContext, _previous_action_payload: Any |
5960
"module_name": render_context.module_name,
6061
"memory_folder": memory_folder,
6162
"memory_file_names": MemoryManager.list_memory_files(memory_folder),
63+
"test_script_timeout_seconds": render_utils.effective_test_script_timeout(render_context),
64+
"sandbox_contract": build_sandbox_contract(render_context),
6265
}
6366
# Orientation seeds: codebase map + per-FRID implementation history. Built after
6467
# revert_changes_for_frid so the map reflects the baseline the agent will see.

‎render_machine/actions/commit_conformance_tests_changes.py‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,18 @@ def __init__(self, implementation_code_commit_message: str, conformance_tests_co
1515
self.conformance_tests_commit_message = conformance_tests_commit_message
1616

1717
def execute(self, render_context: RenderContext, _previous_action_payload: Any | None):
18+
# Approved conformance fixes are committed at approval time, so the build
19+
# folder can be clean here even though the implementation DID change during
20+
# conformance fixing. The marker commit must still be made (possibly empty)
21+
# and the "implementation updated" outcome must still fire: ambiguity
22+
# analysis diffs the implementation commit against this marker commit, a
23+
# range that includes the approval commits in between.
24+
implementation_changed_by_fixes = (
25+
render_context.conformance_tests_running_context is not None
26+
and render_context.conformance_tests_running_context.implementation_committed_on_fix_approval
27+
)
1828
implementation_updated = False
19-
if git_utils.is_dirty(render_context.build_folder):
29+
if git_utils.is_dirty(render_context.build_folder) or implementation_changed_by_fixes:
2030
git_utils.add_all_files_and_commit(
2131
render_context.build_folder,
2232
self.implementation_code_commit_message,

0 commit comments

Comments
 (0)