Skip to content

Commit 23ab10a

Browse files
committed
Give unit-test fixing access to memory, kept separate by test surface
The unit-test fix loop runs up to 20 attempts and produced no memory and consumed none, even though it is doing the same thing the conformance loop does: applying a change and observing what a test run says about it. Both loops now write and read memory, and the two are kept distinct by the `suite` field the record schema already carried. Writing: - run_unit_tests records the observation after each run, pairing the fix applied before it with the outcome, exactly as the conformance path does - UnitTestsRunningContext carries the pending intervention across the run - records are written with suite=unittest, so the surface is visible both in the record and in the file name (unittest-<fingerprint>-<hash>.json) Reading: - fix_unittests_issue now carries memory_files_content, and the unittest prompt template declares the variable. The field is optional server-side, so a client that predates this still validates and MIN_CLIENT_VERSION does not move - the memory section sits after the template's cache breakpoint, since retrieved records change between attempts Suite separation in retrieval: - records from another surface rank below every same-surface match rather than being hidden. A unit-test failure and a conformance failure never share a fingerprint (different runners, different output), but they do share the files an intervention touched, and that is worth surfacing once the same-surface evidence runs out - the prompt explains what scope.suite means and that the current surface's observations are the more relevant ones One honest gap: unit-test fixes arrive as a single set of changed files with no indication of which are implementation and which are unit tests, so the intervention target is recorded as UNCLASSIFIED and the touched_* flags are left undetermined. Deciding it from file paths would be a guess, and the conformance path gets this for free from issue_reason_code. To keep "not determined" distinct from a determined "no", those flags are now Optional and default to None. This also means the test-weakening flag stays a conformance-only signal -- which is right, since generated unit tests live inside the module and editing them is normal rather than suspect.
1 parent dd183f2 commit 23ab10a

11 files changed

Lines changed: 281 additions & 18 deletions

File tree

‎codeplain_REST_api.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,7 @@ def fix_unittests_issue(
233233
plain_source_tree,
234234
linked_resources,
235235
existing_files_content,
236+
memory_files_content,
236237
module_name: str,
237238
required_modules,
238239
unittests_issue,
@@ -246,6 +247,7 @@ def fix_unittests_issue(
246247
"plain_source_tree": plain_source_tree,
247248
"linked_resources": linked_resources,
248249
"existing_files_content": existing_files_content,
250+
"memory_files_content": memory_files_content,
249251
"module_name": module_name,
250252
"required_modules": required_modules,
251253
"unittests_issue": unittests_issue,

‎memory_management/record.py‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,10 @@ class InterventionTarget(str, Enum):
5555
IMPLEMENTATION = "IMPLEMENTATION"
5656
CONFORMANCE_TESTS = "CONFORMANCE_TESTS"
5757
NONE = "NONE"
58+
# The changed files are recorded, but which of them are implementation and which
59+
# are tests is not known. Unit-test fixes land in one response with no such split,
60+
# and classifying by file path would be a guess rather than an observation.
61+
UNCLASSIFIED = "UNCLASSIFIED"
5862

5963

6064
class AttributionConfidence(str, Enum):
@@ -134,8 +138,9 @@ class Intervention:
134138
target: str
135139
files_changed: list[str] = field(default_factory=list)
136140
lines_changed: int = 0
137-
touched_implementation: bool = False
138-
touched_test_files: bool = False
141+
# ``None`` means not determined, which is different from a determined ``False``.
142+
touched_implementation: Optional[bool] = None
143+
touched_test_files: Optional[bool] = None
139144

140145
def signature(self) -> str:
141146
"""Stable identity of this intervention, used to deduplicate records."""

‎memory_management/retrieval.py‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,12 @@
3030
_DEPTH_BY_ATTEMPTS = ((0, 0), (2, 3), (5, 6))
3131
_MAX_DEPTH = 12
3232

33+
# Records from a different test surface rank below every same-surface match. A unit-test
34+
# failure and a conformance failure never share a fingerprint (different runners, different
35+
# output), but they can share the files an intervention touched, which is worth surfacing
36+
# once the same-surface evidence is exhausted.
37+
_CROSS_SUITE_PENALTY = 10
38+
3339
_BM25_K1 = 1.5
3440
_BM25_B = 0.75
3541
_TOKEN_PATTERN = re.compile(r"[a-z0-9_]+")
@@ -130,6 +136,7 @@ def rank_records(
130136
files_changed: Optional[Iterable[str]] = None,
131137
signature: Optional[str] = None,
132138
mode: MemoryMode = MemoryMode.ALL,
139+
suite: Optional[str] = None,
133140
) -> list[MemoryRecord]:
134141
"""Order records by relevance to the failure currently being worked on."""
135142
allowed = _allowed_statuses(mode)
@@ -148,6 +155,8 @@ def rank_records(
148155
continue
149156
# Lexical-only matches rank behind every symbolic match.
150157
tier = 4
158+
if suite is not None and record.scope.suite != suite:
159+
tier += _CROSS_SUITE_PENALTY
151160
# More occurrences means the observation repeated within this render, which is
152161
# independent evidence; ties break toward it.
153162
scored.append((tier, -lexical_score, -record.occurrences, record.memory_id, record))
@@ -164,6 +173,7 @@ def select_records(
164173
files_changed: Optional[Iterable[str]] = None,
165174
signature: Optional[str] = None,
166175
mode: MemoryMode = MemoryMode.ALL,
176+
suite: Optional[str] = None,
167177
) -> list[MemoryRecord]:
168178
"""Rank, then cut to the adaptive depth."""
169179
if depth <= 0 or mode is MemoryMode.OFF:
@@ -176,5 +186,6 @@ def select_records(
176186
files_changed=files_changed,
177187
signature=signature,
178188
mode=mode,
189+
suite=suite,
179190
)
180191
return ranked[:depth]

‎memory_management/store.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ def retrieve(
6868
files_changed: Optional[list[str]] = None,
6969
signature: Optional[str] = None,
7070
fix_attempts: int = 0,
71+
suite: Optional[str] = None,
7172
) -> dict[str, str]:
7273
"""Select the records relevant to the current failure.
7374
@@ -85,6 +86,7 @@ def retrieve(
8586
files_changed=files_changed,
8687
signature=signature,
8788
mode=self.memory_mode,
89+
suite=suite,
8890
)
8991
if selected:
9092
console.debug(f"Retrieved {len(selected)} memory record(s) for the current failure.")

‎render_machine/actions/fix_conformance_test.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ def execute(self, render_context: RenderContext, previous_action_payload: Any |
6767
existing_files, existing_files_content = ImplementationCodeHelpers.fetch_existing_files(
6868
render_context.build_folder
6969
)
70-
memory_files_content = render_context.retrieve_memory_for_current_failure(previous_conformance_tests_issue)
70+
memory_files_content = render_context.retrieve_memory_for_conformance_failure(previous_conformance_tests_issue)
7171
(
7272
existing_conformance_test_files,
7373
existing_conformance_test_files_content,

‎render_machine/actions/fix_unit_tests.py‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,15 @@
11
from typing import Any
22

3+
import diff_utils
34
import file_utils
45
import render_machine.render_utils as render_utils
6+
from memory_management import InterventionTarget
57
from plain2code_console import console
68
from plain2code_exceptions import InternalClientError
79
from render_machine.actions.base_action import BaseAction
810
from render_machine.implementation_code_helpers import ImplementationCodeHelpers
911
from render_machine.render_context import RenderContext
12+
from render_machine.render_types import PendingIntervention
1013

1114
MAX_ISSUE_LENGTH = 10000
1215

@@ -32,11 +35,14 @@ def execute(self, render_context: RenderContext, previous_action_payload: Any |
3235

3336
render_utils.print_inputs(render_context, existing_files_content, "Files sent as input to unit tests fixing:")
3437

38+
memory_files_content = render_context.retrieve_memory_for_unittest_failure(previous_unittests_issue)
39+
3540
response_files = render_context.codeplain_api.fix_unittests_issue(
3641
render_context.frid_context.frid,
3742
render_context.plain_source_tree,
3843
render_context.frid_context.linked_resources,
3944
existing_files_content,
45+
memory_files_content,
4046
render_context.module_name,
4147
render_context.get_required_modules_functionalities(),
4248
previous_unittests_issue,
@@ -48,7 +54,31 @@ def execute(self, render_context: RenderContext, previous_action_payload: Any |
4854
)
4955

5056
render_context.unit_tests_running_context.changed_files.update(changed_files)
57+
self._remember_intervention(render_context, response_files, existing_files_content, previous_unittests_issue)
5158

5259
console.print_files("Files fixed:", render_context.build_folder, response_files, style=console.OUTPUT_STYLE)
5360

5461
return self.SUCCESSFUL_OUTCOME, None
62+
63+
@staticmethod
64+
def _remember_intervention(
65+
render_context: RenderContext,
66+
response_files: dict,
67+
existing_files_content: dict,
68+
failure_output: str,
69+
) -> None:
70+
"""Record what was just changed, to be paired with the next unit-test run's outcome.
71+
72+
The target is left unclassified: a unit-test fix arrives as one set of files with no
73+
indication of which are implementation and which are tests, and deciding that from
74+
file paths would be a guess rather than an observation.
75+
"""
76+
code_diff_files = diff_utils.get_code_diff(response_files, existing_files_content)
77+
running_context = render_context.unit_tests_running_context
78+
running_context.pending_intervention = PendingIntervention(
79+
attempt_index=running_context.fix_attempts,
80+
target=InterventionTarget.UNCLASSIFIED.value,
81+
files_changed=sorted(code_diff_files.keys()),
82+
lines_changed=sum(len(diff.splitlines()) for diff in code_diff_files.values()),
83+
failure_output=failure_output,
84+
)

‎render_machine/actions/run_unit_tests.py‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
from typing import Any
33

44
import render_machine.render_utils as render_utils
5+
from memory_management import Failure, Intervention, Scope, Suite, fingerprint_output
56
from plain2code_console import console
67
from render_machine.actions.base_action import BaseAction
78
from render_machine.render_context import RenderContext
@@ -31,6 +32,9 @@ def execute(self, render_context: RenderContext, _previous_action_payload: Any |
3132

3233
render_context.script_execution_history.latest_unit_test_output_path = unittests_temp_log_file_path
3334
render_context.script_execution_history.should_update_script_outputs = True
35+
36+
self._record_observation(render_context, exit_code, unittests_issue)
37+
3438
if exit_code == 0:
3539
return self.SUCCESSFUL_OUTCOME, None
3640

@@ -48,3 +52,46 @@ def execute(self, render_context: RenderContext, _previous_action_payload: Any |
4852
)
4953
else:
5054
return self.FAILED_OUTCOME, {"previous_unittests_issue": unittests_issue}
55+
56+
@staticmethod
57+
def _record_observation(render_context: RenderContext, exit_code: int, unittests_issue: str) -> None:
58+
"""Log what the fix applied before this run actually did.
59+
60+
Deterministic - no API call, no credit. Nothing is recorded until a fix has been
61+
applied, because a record describes the effect of a change: the first failure of a
62+
functionality is context the render already has.
63+
"""
64+
running_context = render_context.unit_tests_running_context
65+
if running_context is None or running_context.pending_intervention is None:
66+
return
67+
68+
pending = running_context.pending_intervention
69+
running_context.pending_intervention = None
70+
71+
fingerprint_before, signature_before, excerpt_before = fingerprint_output(pending.failure_output)
72+
fingerprint_after, _, _ = fingerprint_output(unittests_issue if exit_code != 0 else None)
73+
74+
render_context.memory_store.record_observation(
75+
scope=Scope(
76+
module=render_context.module_name,
77+
frid=render_context.frid_context.frid,
78+
testing_module=render_context.module_name,
79+
testing_frid=render_context.frid_context.frid,
80+
suite=Suite.UNITTEST.value,
81+
),
82+
failure=Failure(
83+
fingerprint=fingerprint_before,
84+
signature=signature_before,
85+
excerpt=excerpt_before,
86+
exit_code=1,
87+
),
88+
intervention=Intervention(
89+
attempt_index=pending.attempt_index,
90+
target=pending.target,
91+
files_changed=sorted(pending.files_changed),
92+
lines_changed=pending.lines_changed,
93+
),
94+
exit_code_after=exit_code,
95+
fingerprint_after=fingerprint_after,
96+
render_id=render_context.run_state.render_id,
97+
)

‎render_machine/render_context.py‎

Lines changed: 48 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
import plain_spec
88
from codeplain_REST_api import CodeplainAPI
99
from event_bus import EventBus
10-
from memory_management import MemoryStore, fingerprint_output
10+
from memory_management import MemoryStore, Suite, fingerprint_output
1111
from plain2code_console import RETRY_COLOR, console
1212
from plain2code_events import RenderContextSnapshot
1313
from plain2code_state import RunState
@@ -123,29 +123,63 @@ def create_snapshot(self) -> RenderContextSnapshot:
123123
module_name=self.module_name,
124124
)
125125

126-
def retrieve_memory_for_current_failure(self, failure_output: Optional[str]) -> dict[str, str]:
127-
"""Select the memory records relevant to the failure currently being fixed.
126+
def retrieve_memory_for_conformance_failure(self, failure_output: Optional[str]) -> dict[str, str]:
127+
"""Select memory records relevant to the conformance failure being fixed."""
128+
running_context = self.conformance_tests_running_context
129+
130+
return self._retrieve_memory(
131+
failure_output,
132+
suite=Suite.CONFORMANCE.value,
133+
test_name=(
134+
running_context.get_current_conformance_test_folder_name()
135+
if running_context and running_context.current_conformance_tests_exist()
136+
else None
137+
),
138+
fix_attempts=running_context.fix_attempts if running_context else 0,
139+
)
128140

129-
The retrieval query comes entirely from render state - the failure being worked on,
130-
the test that produced it, and how many fix attempts have been made - so no planner
131-
call is needed. Retrieval depth grows with the attempt count: an early attempt gets
132-
little or nothing, a deep fix loop gets the full picture.
141+
def retrieve_memory_for_unittest_failure(self, failure_output: Optional[str]) -> dict[str, str]:
142+
"""Select memory records relevant to the unit-test failure being fixed.
143+
144+
Unit-test observations rank first; conformance observations can still surface once
145+
the same-surface evidence is exhausted, since both loops change the same code.
146+
"""
147+
running_context = self.unit_tests_running_context
148+
149+
return self._retrieve_memory(
150+
failure_output,
151+
suite=Suite.UNITTEST.value,
152+
files_changed=sorted(running_context.changed_files) if running_context else None,
153+
fix_attempts=running_context.fix_attempts if running_context else 0,
154+
)
155+
156+
def _retrieve_memory(
157+
self,
158+
failure_output: Optional[str],
159+
suite: str,
160+
test_name: Optional[str] = None,
161+
files_changed: Optional[list[str]] = None,
162+
fix_attempts: int = 0,
163+
) -> dict[str, str]:
164+
"""Query the memory store for the failure currently being fixed.
165+
166+
The query comes entirely from render state - the failure being worked on, the test
167+
that produced it, and how many fix attempts have been made - so no planner call is
168+
needed. Depth grows with the attempt count: an early attempt gets little or nothing,
169+
a deep fix loop gets the full picture.
133170
"""
134171
if failure_output is None:
135172
return {}
136173

137174
fingerprint, signature, _ = fingerprint_output(failure_output)
138-
running_context = self.conformance_tests_running_context
139175

140176
return self.memory_store.retrieve(
141177
fingerprint=fingerprint,
142-
test_name=(
143-
running_context.get_current_conformance_test_folder_name()
144-
if running_context and running_context.current_conformance_tests_exist()
145-
else None
146-
),
178+
test_name=test_name,
179+
files_changed=files_changed,
147180
signature=signature,
148-
fix_attempts=running_context.fix_attempts if running_context else 0,
181+
fix_attempts=fix_attempts,
182+
suite=suite,
149183
)
150184

151185
def get_required_modules_functionalities(self):

‎render_machine/render_types.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ class FridContext:
7373
class UnitTestsRunningContext:
7474
fix_attempts: int
7575
changed_files: set[str] = field(default_factory=set)
76+
pending_intervention: Optional[PendingIntervention] = None
7677

7778

7879
class ConformanceTestsRunningContext:

0 commit comments

Comments
 (0)