Repository navigation
perf(experience): read the incubation scope-head snapshot once per window - #2225
Inference1 wants to merge 2 commits into
Conversation
…ndow An outcome without a handoff receipt matches against every active Experience, and every such row reloaded that snapshot with one get() per Experience. Load it once per window and batch the revision reads with get_many; the window writes no artifacts, so each row sees the same heads. Measured with 40 Experiences and four outcome rows: 355 -> 34 statements.
hidb4ai
left a comment
hidb4ai
left a comment
There was a problem hiding this comment.
Reviewed commit 8d87321edd346d5dd83297e9fbc04330d44602a6; no correctness findings in the snapshot reuse or batched reads. SQLite differential probes preserved matches, observations, proposals, and replay behavior, including missing citations and batches over 100 revisions.
One validation issue remains: code-seekdb (3.14) timed out after 10 minutes in the embedded SeekDB migration step, while running test_real_mysql_recovery_upgrade_preserves_receipt_and_resumes_registered_ddl_prefix[True-seekdb]. Please rerun or investigate that check before merging. The log does not establish that this failure was caused by this PR; I did not independently run the OceanBase/SeekDB acceptance tests.
|
Thanks for the review. On the
This PR changes only I do not have write access to re-run the job. Could you re-run |
The code-seekdb (3.14) job timed out in the embedded SeekDB migration step; the same step also timed out on unrelated branches that day.
Which issue or RFC does this PR close?
No issue to close. Found while reviewing the Experience incubation read path.
Rationale for this change
RelationalRecurrenceLedger.record_windowhandles every task-outcome row without a handoff receipt by matching it against all active Experiences of the Scope. Each such row re-read that snapshot with oneArtifactRepository.getper Experience, so a window costrows x Experiences x 2statements and grew with both dimensions.Measured on the recurrence ledger with the scenario from
tests/e2e/test_recurring_failure_repair.py(statements counted at the engine level):What changes are included in this PR?
record_windowparses the outcomes once, reads the scope-head snapshot once when any row is unlinked, and passes it to_record_outcome. This method writes no Artifacts, so one snapshot per window returns the same data that every row used to re-read._contentsloads the cited revisions with oneget_manybatch.get_manyis all-or-nothing, soRepositoryNotFoundErrorfalls back to the previous per-reference skip loop.test_incubation_statement_count_does_not_grow_with_scope_experiences: equal counts for 10 and 40 active Experiences, with a ceiling one statement above the measured value.Are there any user-facing changes?
No. The recurrence proposals, matches and observations are unchanged.
How was this change tested?
pytest tests/builtin/runtime/test_recurrence_ledger.py tests/e2e/test_recurring_failure_repair.py -q-> 16 passed.pytest tests/e2e/test_portable_archive.py::test_restore_rejects_invalid_remote_source_dependencies_before_writing -q-> 8 passed.assert 31 == 91) and passes with the change.ruff checkandruff format --checkpass on both changed files.