Skip to content

Commit 7c94505

Browse files
committed
Consolidate the fix journal into lessons instead of rebuilding memory each round
Removes the per-run memory call and replaces it with a single consolidation when a functionality's conformance tests pass, reading the journal that the fix loop has been accumulating. The lessons land where memory files are already read from, so they flow into every later prompt through the existing path - only the producer changes. A functionality that passed without any fixing has an empty journal and costs no call at all. The journal is discarded once its lessons have been taken. The three previous_conformance_tests_issue fields on the running context go with it; they existed only to feed the per-run memory call.
1 parent dc112fb commit 7c94505

6 files changed

Lines changed: 176 additions & 107 deletions

File tree

‎codeplain_REST_api.py‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -254,40 +254,34 @@ def fix_unittests_issue(
254254

255255
return self.post_request(endpoint_url, headers, payload, run_state)
256256

257-
def create_conformance_test_memory(
257+
def consolidate_conformance_test_lessons(
258258
self,
259259
frid,
260260
plain_source_tree,
261261
linked_resources,
262-
existing_files_content,
263262
memory_files_content,
264263
module_name,
265264
required_modules,
266-
code_diff,
267265
conformance_tests_files,
268266
acceptance_tests,
269-
conformance_tests_issue,
270267
conformance_tests_folder_name,
271-
previous_conformance_tests_issue_old,
268+
conformance_test_journal,
272269
run_state: RunState,
273270
):
274-
endpoint_url = f"{self.api_url}/create_conformance_test_memory"
271+
endpoint_url = f"{self.api_url}/consolidate_conformance_test_lessons"
275272
headers = {"X-API-Key": self.api_key, "Content-Type": "application/json"}
276273

277274
payload = {
278275
"frid": frid,
279276
"plain_source_tree": plain_source_tree,
280277
"linked_resources": linked_resources,
281-
"existing_files_content": existing_files_content,
282278
"memory_files_content": memory_files_content,
283279
"module_name": module_name,
284280
"required_modules": required_modules,
285-
"code_diff": code_diff,
286281
"conformance_tests_files": conformance_tests_files,
287282
"acceptance_tests": acceptance_tests,
288-
"conformance_tests_issue": conformance_tests_issue,
289283
"conformance_tests_folder_name": conformance_tests_folder_name,
290-
"previous_conformance_tests_issue": previous_conformance_tests_issue_old,
284+
"conformance_test_journal": conformance_test_journal,
291285
}
292286

293287
return self.post_request(endpoint_url, headers, payload, run_state)

‎memory_management.py‎

Lines changed: 46 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,24 @@
1-
import json
1+
"""What the renderer remembers about conformance tests, and when.
2+
3+
Two artifacts, with different lifetimes:
4+
5+
* The **journal**, written mechanically as a functionality's fix loop runs, holding every attempt made against
6+
it. See :mod:`conformance_test_journal`. It is discarded once that functionality's tests pass.
7+
8+
* The **lessons**, extracted from the journal at that point and kept for the rest of the project. These are
9+
the memory files, and they are fed into every later prompt - which is why the set is capped and why a lesson
10+
has to be a transferable constraint rather than the story of one bug.
11+
12+
Consolidation runs once per functionality. It replaces an earlier arrangement that rebuilt a memory after
13+
every conformance test run and, being told not to duplicate itself, deleted the previous one each time - so a
14+
twenty round fix loop remembered only its most recent round.
15+
"""
16+
217
import os
318

419
import file_utils
20+
from conformance_test_journal import ConformanceTestJournal
521
from plain2code_console import console
6-
from render_machine.implementation_code_helpers import ImplementationCodeHelpers
722
from render_machine.render_context import RenderContext
823

924
CONFORMANCE_TESTS_SUCCESS_EXIT_CODE = 0
@@ -27,106 +42,58 @@ def __init__(self, codeplain_api, memory_folder: str):
2742
self.codeplain_api = codeplain_api
2843
self.memory_folder = memory_folder
2944

30-
def create_conformance_tests_memory(
31-
self, render_context: RenderContext, exit_code: int, conformance_tests_issue: str
32-
):
45+
def consolidate_lessons(self, render_context: RenderContext) -> None:
46+
"""Extract what transfers to later functionalities, then discard the journal it came from.
3347
34-
current_conformance_tests_issue_frid = render_context.conformance_tests_running_context.current_testing_frid
35-
current_conformance_tests_issue_module = (
36-
render_context.conformance_tests_running_context.current_testing_module_name
37-
)
38-
old_conformance_tests_issue_frid = (
39-
render_context.conformance_tests_running_context.previous_conformance_tests_issue_frid
40-
)
41-
old_conformance_tests_issue_module = (
42-
render_context.conformance_tests_running_context.previous_conformance_tests_issue_module
43-
)
44-
45-
old_conformance_tests_issue = (
46-
render_context.conformance_tests_running_context.previous_conformance_tests_issue_old
47-
)
48-
49-
is_first_time_running_conformance_tests = (
50-
old_conformance_tests_issue_frid is None
51-
or old_conformance_tests_issue_frid == ""
52-
or old_conformance_tests_issue_module != current_conformance_tests_issue_module
53-
)
54-
is_same_frid_as_previous_failing_test = (
55-
current_conformance_tests_issue_frid == old_conformance_tests_issue_frid
56-
and current_conformance_tests_issue_module == old_conformance_tests_issue_module
57-
)
58-
is_conformance_test_failed = exit_code != CONFORMANCE_TESTS_SUCCESS_EXIT_CODE
48+
Called when a functionality's own conformance tests pass. A functionality whose tests passed without
49+
any fixing has an empty journal and teaches nothing, so no call is made for it.
50+
"""
51+
ctx = render_context.conformance_tests_running_context
52+
if ctx.current_testing_frid is None:
53+
return
5954

60-
should_create_memory = not is_first_time_running_conformance_tests and (
61-
is_same_frid_as_previous_failing_test or is_conformance_test_failed
55+
journal = ConformanceTestJournal.load(
56+
self.memory_folder, ctx.current_testing_module_name, ctx.current_testing_frid
6257
)
63-
code_diff_files = render_context.conformance_tests_running_context.code_diff_files
64-
65-
if not should_create_memory or code_diff_files is None:
66-
console.debug(
67-
"Skipping creation of conformance test memory because the conditions for creating memories are not met."
68-
)
58+
previous_attempts = journal.render_for_prompt()
59+
if not previous_attempts:
60+
console.debug("No conformance test fixes were needed, so there are no lessons to consolidate.")
6961
return
7062

71-
existing_files, existing_files_content = ImplementationCodeHelpers.fetch_existing_files(
72-
render_context.build_folder
73-
)
7463
memory_files, memory_files_content = MemoryManager.fetch_memory_files(self.memory_folder)
75-
76-
conformance_tests_folder_name = (
77-
render_context.conformance_tests_running_context.get_current_conformance_test_folder_name()
78-
)
79-
64+
conformance_tests_folder_name = ctx.get_current_conformance_test_folder_name()
8065
(
8166
_,
82-
existing_conformance_test_files_content,
67+
conformance_tests_files_content,
8368
) = render_context.conformance_tests.fetch_existing_conformance_test_files(
8469
render_context.module_name,
8570
render_context.required_modules,
86-
render_context.conformance_tests_running_context.current_testing_module_name,
71+
ctx.current_testing_module_name,
8772
conformance_tests_folder_name,
8873
)
89-
acceptance_tests = render_context.conformance_tests_running_context.get_current_acceptance_tests()
9074

91-
response_files = render_context.codeplain_api.create_conformance_test_memory(
75+
console.info(
76+
f"Consolidating what fixing the conformance tests for functionality {ctx.current_testing_frid} "
77+
f"in module {ctx.current_testing_module_name} has taught."
78+
)
79+
80+
response_files = render_context.codeplain_api.consolidate_conformance_test_lessons(
9281
render_context.frid_context.frid,
9382
render_context.plain_source_tree,
9483
render_context.frid_context.linked_resources,
95-
existing_files_content,
9684
memory_files_content,
9785
render_context.module_name,
9886
render_context.get_required_modules_functionalities(),
99-
code_diff_files,
100-
existing_conformance_test_files_content,
101-
acceptance_tests,
102-
conformance_tests_issue,
87+
conformance_tests_files_content,
88+
ctx.get_current_acceptance_tests(),
10389
conformance_tests_folder_name,
104-
old_conformance_tests_issue,
90+
previous_attempts,
10591
run_state=render_context.run_state,
10692
)
107-
if len(response_files) > 0:
93+
94+
if response_files:
10895
memory_folder_path = os.path.join(self.memory_folder, CONFORMANCE_TEST_MEMORY_SUBFOLDER)
10996
file_utils.store_response_files(memory_folder_path, response_files, memory_files)
11097

111-
def delete_unresolved_memory_files(self):
112-
"""Delete memory files whose resolution_status is not 'RESOLVED'."""
113-
memory_path = os.path.join(self.memory_folder, CONFORMANCE_TEST_MEMORY_SUBFOLDER)
114-
if not os.path.exists(memory_path):
115-
return
116-
117-
memory_files = file_utils.list_all_text_files(memory_path)
118-
for file_name in memory_files:
119-
file_path = os.path.join(memory_path, file_name)
120-
try:
121-
with open(file_path, "r") as f:
122-
content = json.load(f)
123-
if content.get("resolution_status") == "RESOLVED":
124-
continue
125-
else:
126-
os.remove(file_path)
127-
except (json.JSONDecodeError, OSError):
128-
# Not a valid JSON file, unlikely to be a valid memory file, delete it
129-
console.error(f"Memory file is not a valid JSON file: {file_name}. Deleting it.")
130-
os.remove(file_path)
131-
132-
console.debug(f"Deleted temporary memory file: {file_name}")
98+
# The journal has served its purpose for this functionality; anything worth keeping is now a lesson.
99+
journal.delete(self.memory_folder)

‎render_machine/actions/fix_conformance_test.py‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -91,16 +91,6 @@ def execute(self, render_context: RenderContext, previous_action_payload: Any |
9191
)
9292
previous_conformance_tests_issue = previous_action_payload["previous_conformance_tests_issue"]
9393

94-
render_context.conformance_tests_running_context.previous_conformance_tests_issue_old = (
95-
previous_conformance_tests_issue
96-
)
97-
render_context.conformance_tests_running_context.previous_conformance_tests_issue_frid = (
98-
render_context.conformance_tests_running_context.current_testing_frid
99-
)
100-
render_context.conformance_tests_running_context.previous_conformance_tests_issue_module = (
101-
render_context.conformance_tests_running_context.current_testing_module_name
102-
)
103-
10494
existing_files, existing_files_content = ImplementationCodeHelpers.fetch_existing_files(
10595
render_context.build_folder
10696
)

‎render_machine/actions/run_conformance_tests.py‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -84,18 +84,14 @@ def execute(self, render_context: RenderContext, _previous_action_payload: Any |
8484

8585
self._fingerprint_run(render_context, exit_code, conformance_tests_issue)
8686

87-
render_context.memory_manager.create_conformance_tests_memory(
88-
render_context, exit_code, conformance_tests_issue
89-
)
90-
9187
if exit_code == 0:
9288
if (
9389
render_context.conformance_tests_running_context.current_testing_module_name
9490
== render_context.module_name
9591
and render_context.conformance_tests_running_context.current_testing_frid
9692
== render_context.frid_context.frid
9793
):
98-
render_context.memory_manager.delete_unresolved_memory_files()
94+
render_context.memory_manager.consolidate_lessons(render_context)
9995
return self.SUCCESSFUL_OUTCOME, None
10096

10197
if exit_code in UNRECOVERABLE_ERROR_EXIT_CODES:

‎render_machine/render_types.py‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -96,9 +96,6 @@ def __init__(
9696
# repeat of an earlier one.
9797
self.last_failure_signature: Optional[str] = None
9898
self.last_failure_excerpt: Optional[str] = None
99-
self.previous_conformance_tests_issue_old: Optional[str] = None
100-
self.previous_conformance_tests_issue_frid: Optional[str] = None
101-
self.previous_conformance_tests_issue_module: Optional[str] = None
10299
self.code_diff_files: Optional[dict[str, str]] = None
103100

104101
def get_conformance_tests_json(self, module_name: str) -> dict:

‎tests/test_lesson_consolidation.py‎

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
"""Tests for consolidating a functionality's fix journal into lessons once its tests pass."""
2+
3+
import os
4+
from unittest.mock import MagicMock
5+
6+
import pytest
7+
8+
from conformance_test_journal import TARGET_IMPLEMENTATION, ConformanceTestJournal
9+
from memory_management import CONFORMANCE_TEST_MEMORY_SUBFOLDER, MemoryManager
10+
from render_machine.render_types import ConformanceTestsRunningContext
11+
12+
LESSONS_FILE_NAME = "conformance_test_lessons.json"
13+
14+
15+
@pytest.fixture
16+
def render_context(tmp_path):
17+
context = MagicMock()
18+
context.memory_manager.memory_folder = str(tmp_path)
19+
context.module_name = "module"
20+
context.frid_context.frid = "1"
21+
context.conformance_tests_running_context = ConformanceTestsRunningContext(
22+
current_testing_module_name="module",
23+
current_testing_frid="1",
24+
fix_attempts=0,
25+
conformance_tests_json={"1": {"folder_name": "tests_for_1"}},
26+
conformance_tests_render_attempts=0,
27+
current_testing_frid_specifications=None,
28+
should_prepare_testing_environment=False,
29+
)
30+
context.conformance_tests.fetch_existing_conformance_test_files.return_value = ([], {})
31+
context.codeplain_api.consolidate_conformance_test_lessons.return_value = {
32+
LESSONS_FILE_NAME: '{"lessons": [{"lesson": "a rule"}]}'
33+
}
34+
return context
35+
36+
37+
@pytest.fixture
38+
def memory_manager(render_context, tmp_path):
39+
return MemoryManager(render_context.codeplain_api, str(tmp_path))
40+
41+
42+
def _write_journal(tmp_path, rounds=2):
43+
journal = ConformanceTestJournal("module", "1")
44+
for index in range(rounds):
45+
journal.record_attempt(TARGET_IMPLEMENTATION, [f"file_{index}.py"], issue_excerpt="E assert 3 == 4")
46+
journal.save(str(tmp_path))
47+
return journal
48+
49+
50+
def test_lessons_are_consolidated_from_the_journal(memory_manager, render_context, tmp_path):
51+
_write_journal(tmp_path)
52+
53+
memory_manager.consolidate_lessons(render_context)
54+
55+
render_context.codeplain_api.consolidate_conformance_test_lessons.assert_called_once()
56+
journal_argument = render_context.codeplain_api.consolidate_conformance_test_lessons.call_args[0][-1]
57+
assert "Attempt 1" in journal_argument and "Attempt 2" in journal_argument
58+
59+
60+
def test_the_consolidated_lessons_are_stored_where_memory_files_are_read_from(memory_manager, render_context, tmp_path):
61+
_write_journal(tmp_path)
62+
63+
memory_manager.consolidate_lessons(render_context)
64+
65+
_, memory_files_content = MemoryManager.fetch_memory_files(str(tmp_path))
66+
assert LESSONS_FILE_NAME in memory_files_content
67+
68+
69+
def test_the_journal_is_discarded_once_its_lessons_have_been_taken(memory_manager, render_context, tmp_path):
70+
_write_journal(tmp_path)
71+
72+
memory_manager.consolidate_lessons(render_context)
73+
74+
assert not os.path.exists(ConformanceTestJournal.journal_path(str(tmp_path), "module", "1"))
75+
76+
77+
def test_the_lessons_already_on_record_are_sent_so_they_can_be_reconciled(memory_manager, render_context, tmp_path):
78+
_write_journal(tmp_path)
79+
memory_path = os.path.join(str(tmp_path), CONFORMANCE_TEST_MEMORY_SUBFOLDER)
80+
os.makedirs(memory_path, exist_ok=True)
81+
with open(os.path.join(memory_path, LESSONS_FILE_NAME), "w", encoding="utf-8") as recorded:
82+
recorded.write('{"lessons": [{"lesson": "an earlier rule"}]}')
83+
84+
memory_manager.consolidate_lessons(render_context)
85+
86+
sent_memory_files = render_context.codeplain_api.consolidate_conformance_test_lessons.call_args[0][3]
87+
assert "an earlier rule" in sent_memory_files[LESSONS_FILE_NAME]
88+
89+
90+
def test_a_functionality_that_needed_no_fixing_costs_no_call(memory_manager, render_context):
91+
"""An empty journal means the tests passed first time, so there is nothing to have learned."""
92+
memory_manager.consolidate_lessons(render_context)
93+
94+
render_context.codeplain_api.consolidate_conformance_test_lessons.assert_not_called()
95+
96+
97+
def test_nothing_is_consolidated_when_no_functionality_is_under_test(memory_manager, render_context, tmp_path):
98+
_write_journal(tmp_path)
99+
render_context.conformance_tests_running_context.current_testing_frid = None
100+
101+
memory_manager.consolidate_lessons(render_context)
102+
103+
render_context.codeplain_api.consolidate_conformance_test_lessons.assert_not_called()
104+
105+
106+
def test_the_journal_survives_a_consolidation_that_returned_nothing(memory_manager, render_context, tmp_path):
107+
"""Returning no lessons is a valid outcome; it must not be confused with a failure to run."""
108+
_write_journal(tmp_path)
109+
render_context.codeplain_api.consolidate_conformance_test_lessons.return_value = {}
110+
111+
memory_manager.consolidate_lessons(render_context)
112+
113+
_, memory_files_content = MemoryManager.fetch_memory_files(str(tmp_path))
114+
assert memory_files_content == {}
115+
116+
117+
def test_only_the_journal_of_the_functionality_under_test_is_harvested(memory_manager, render_context, tmp_path):
118+
"""Journals are per functionality, so another functionality's record is not swept up by this one."""
119+
_write_journal(tmp_path)
120+
render_context.conformance_tests_running_context.current_testing_frid = "2"
121+
122+
memory_manager.consolidate_lessons(render_context)
123+
124+
render_context.codeplain_api.consolidate_conformance_test_lessons.assert_not_called()
125+
assert os.path.exists(ConformanceTestJournal.journal_path(str(tmp_path), "module", "1"))

0 commit comments

Comments
 (0)