Skip to content

Commit 06a00cf

Browse files
atharvasclaude
andcommitted
feat(overrides): wire expected_n and benchmark_dest, reviving two gates
Both were open producer gaps recorded in the 2026-08-13 ledger: the invariant, the column and the tests all existed, but nothing supplied the input, so each check skipped forever while reading as live. benchmark_dest_missing (FATAL) had been inert since the day it shipped -- nothing in the tree ever set $BENCHMARK_DEST. Now: overrides lookup -> _build_pr_image -> ImageManager.build_pr_image -> BENCHMARK_DEST build arg -> docker_build_run.sh's existing conditional breadcrumb. The ARG is declared in the `run` stage specifically, since ARGs do not cross FROM boundaries and docker_build_run.sh would not otherwise see it. Verified by running the emission block in real containers under all three conditions, then feeding the resulting breadcrumbs through the real evaluator: declared+present -> passes, declared+missing -> FATAL fires (ok=False), undeclared -> skips. That is now pinned as a test; before this change all three cases skipped identically. dilution_ratio (#18) reads FORMULACODE_EXPECTED_N, which nothing injected. Now supplied per task via [verifier.env] from the override row. None injects NOTHING rather than an empty value -- emitting a key that is always empty would make the wiring look live when it is not. The lookup is cached per process. Consumers sit in per-item loops (stage 6 enqueues neighbours mid-flight, so there is no point where the full task set is known), and an uncached read would be one round-trip per task. A failed read caches {} too, since an absent table stays absent. Behaviour change worth knowing: the 5 override tasks now hard-fail stage 6 if their declared benchmark file does not survive git clean. That is the gate doing its job -- it is the joblib/asv_benchmarks.txt wipe it was written for -- but it can reject containers that passed yesterday. Every task without an override row is unaffected. Also fixed: adding the lookup made the "mocked" synthesize_images tests open a REAL Supabase connection, because fetch_all resolves its own client. They passed only because a local DB happened to be running. Now patched. All three producers proven by deletion: removing the ARG, the adapter injection, or the build-arg pass each fails its test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 5a273b4 commit 06a00cf

8 files changed

Lines changed: 508 additions & 2 deletions

File tree

src/datasmith/docker/images.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ def build_pr_image(
117117
commit_sha: str = "HEAD",
118118
env_payload: str = "[]",
119119
py_version: str = "",
120+
benchmark_dest: str = "",
120121
) -> str:
121122
ctx = context or _default_context()
122123
tag = get_pr_image_name(owner, repo, issue_number)
@@ -131,6 +132,11 @@ def build_pr_image(
131132
build_args["BUILD_SCRIPT"] = build_script
132133
if py_version:
133134
build_args["PY_VERSION"] = py_version
135+
if benchmark_dest:
136+
# Only passed when declared. Empty would satisfy the ARG default
137+
# anyway, but passing it explicitly keeps the intent visible in
138+
# `docker history` for tasks that HAVE an override row.
139+
build_args["BENCHMARK_DEST"] = benchmark_dest
134140
self._docker.build(
135141
ctx,
136142
tags=[tag],

src/datasmith/docker/templates/Dockerfile.pr

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,23 @@ RUN chmod +x /profile.sh /run-tests.sh
3838
RUN micromamba clean --all --yes
3939

4040
FROM pkg AS run
41+
# The operator-declared benchmark file this task's measurement depends on,
42+
# from formulacode_task_overrides. docker_build_run.sh turns it into the
43+
# benchmark_dest_present_post_clean breadcrumb, which is the input to the
44+
# FATAL benchmark_dest_missing invariant.
45+
#
46+
# Declared HERE, in the `run` stage, because ARGs do not cross FROM
47+
# boundaries -- docker_build_run.sh runs in this stage and would not see an
48+
# ARG declared anywhere else.
49+
#
50+
# Empty is the normal case (only tasks with an override row declare one), and
51+
# docker_build_run.sh deliberately emits nothing when it is empty, so the
52+
# invariant skips rather than firing against a value nobody declared.
53+
ARG BENCHMARK_DEST=""
4154

4255
COPY docker_build_run.sh /docker_build_run.sh
4356
RUN chmod +x /docker_build_run.sh \
44-
&& /docker_build_run.sh
57+
&& BENCHMARK_DEST="${BENCHMARK_DEST}" /docker_build_run.sh
4558

4659
FROM run AS final
4760
ARG BENCHMARKS=""

src/datasmith/harbor_adapter/adapter.py

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,8 +209,23 @@ def generate_task(
209209
storage: str = "10G",
210210
rounds: int = 1,
211211
verifier_env: dict[str, str] | None = None,
212+
expected_n: int | None = None,
212213
) -> Path:
213-
"""Generate a complete Harbor task directory for the given FormulaCodeRecord."""
214+
"""Generate a complete Harbor task directory for the given FormulaCodeRecord.
215+
216+
``expected_n`` is the operator-declared count of benchmarks this PR
217+
should impact, read from ``formulacode_task_overrides``. It is injected
218+
into the trial container as ``FORMULACODE_EXPECTED_N`` -- the producer
219+
for the ``dilution_ratio`` invariant, which compares the measured
220+
impacted count against it. The container cannot read the overrides
221+
table itself: it is RLS-locked with no ``anon`` grant, and the trial
222+
only carries the anon key.
223+
224+
``None`` injects nothing at all, rather than an empty or zero value.
225+
That is the common case (the column is hand-declared and usually
226+
NULL), and the invariant then skips. Emitting a key that is always
227+
empty would make the wiring look live when it is not.
228+
"""
214229
out_dir = self.out_root / rec.task_dir_name
215230
out_dir.mkdir(parents=True, exist_ok=True)
216231

@@ -222,6 +237,8 @@ def generate_task(
222237

223238
# Generate all task files
224239
self._write_instruction_md(rec, paths)
240+
if expected_n is not None:
241+
verifier_env = {**(verifier_env or {}), "FORMULACODE_EXPECTED_N": str(expected_n)}
225242
self._write_task_toml(rec, paths, timeout_sec, cpus, memory, storage, verifier_env=verifier_env)
226243
self._write_environment_files(rec, paths)
227244
self._write_test_files(rec, paths, run_pytest, rounds)

src/datasmith/runners/harbor_healthcheck.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
from datasmith.harbor_adapter import FormulaCodeAdapter, to_record
2323
from datasmith.utils import get_client, get_logger
24+
from datasmith.utils.overrides import expected_n_for, fetch_overrides
2425

2526
logger = get_logger("runners.harbor_healthcheck")
2627

@@ -96,6 +97,20 @@ def _materialize_tasks(
9697
adapter = FormulaCodeAdapter(harbor_tasks_root=task_dir, force=True)
9798
verifier_env = _build_verifier_env() or None
9899

100+
# Per-task operator declarations. expected_n is the producer for the
101+
# dilution_ratio invariant; the trial container cannot read the table
102+
# itself (RLS-locked, no anon grant), so it is injected per task via
103+
# [verifier.env]. Tasks without a declaration get None and the invariant
104+
# skips, which is the common case.
105+
overrides = fetch_overrides([
106+
(pr["owner"], pr["repo"], int(pr["issue_number"]))
107+
for pr in items
108+
if pr.get("owner") and pr.get("repo") and pr.get("issue_number") is not None
109+
])
110+
if overrides:
111+
n_expected = sum(1 for row in overrides.values() if row.get("expected_n") is not None)
112+
logger.info("Loaded %d task override(s); %d declare expected_n", len(overrides), n_expected)
113+
99114
task_id_map: dict[str, dict[str, Any]] = {}
100115
for pr in items:
101116
try:
@@ -114,6 +129,7 @@ def _materialize_tasks(
114129
run_pytest=True,
115130
rounds=rounds,
116131
verifier_env=verifier_env,
132+
expected_n=expected_n_for(overrides, (rec.owner, rec.repo, rec.issue_number)),
117133
)
118134
except Exception:
119135
logger.exception("generate_task failed for %s/%s#%d", rec.owner, rec.repo, rec.issue_number)

src/datasmith/runners/synthesize_images.py

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
from datasmith.agents.synthesizer import Synthesizer
1212
from datasmith.runners.base import BaseRunner
1313
from datasmith.utils import get_client, get_logger
14+
from datasmith.utils.overrides import benchmark_dest_for, fetch_overrides
1415

1516
logger = get_logger("runners.synthesize_images")
1617

@@ -71,9 +72,16 @@ def _build_pr_image(
7172
docker_context: Any | None = None,
7273
python_version: str = "",
7374
base_sha: str = "",
75+
benchmark_dest: str = "",
7476
) -> str:
7577
"""Build the final PR image from synthesized context (no push).
7678
79+
``benchmark_dest`` is the operator-declared benchmark file from
80+
formulacode_task_overrides. It reaches docker_build_run.sh as the
81+
BENCHMARK_DEST build arg and becomes the input to the FATAL
82+
benchmark_dest_missing invariant. Empty (the common case) means no
83+
override row declared one, and the invariant skips.
84+
7785
Returns the PR image tag that will be used for the subsequent push.
7886
"""
7987
from datasmith.docker.images import ImageManager, get_pr_image_name
@@ -98,6 +106,7 @@ def _build_pr_image(
98106
commit_sha=checkout_sha or "HEAD",
99107
env_payload=env_payload or "[]",
100108
py_version=python_version,
109+
benchmark_dest=benchmark_dest,
101110
)
102111
else:
103112
mgr.build_pr_image(
@@ -107,6 +116,7 @@ def _build_pr_image(
107116
commit_sha=checkout_sha or "HEAD",
108117
env_payload=env_payload or "[]",
109118
py_version=python_version,
119+
benchmark_dest=benchmark_dest,
110120
)
111121

112122
return pr_tag
@@ -507,6 +517,20 @@ async def _do_process_item(self, item: Any) -> None:
507517
sha = item.get("sha", "")
508518
base_sha = item.get("base_sha", "")
509519
solution_patch = item.get("patch", "") or ""
520+
# Operator-declared benchmark file, if this task has an override row.
521+
# Producer for the FATAL benchmark_dest_missing invariant, which has
522+
# been inert since it shipped because nothing set $BENCHMARK_DEST.
523+
# Absent -> "" -> docker_build_run.sh emits no breadcrumb -> the
524+
# invariant skips, exactly as it does today for every task.
525+
benchmark_dest = benchmark_dest_for(fetch_overrides([(owner, repo, issue_number)]), (owner, repo, issue_number))
526+
if benchmark_dest:
527+
logger.info(
528+
"Task override for %s/%s#%d declares benchmark_dest=%s",
529+
owner,
530+
repo,
531+
issue_number,
532+
benchmark_dest,
533+
)
510534
env_payload = item.get("env_payload", "")
511535

512536
from datasmith.docker.images import get_repo_image_name
@@ -544,6 +568,7 @@ async def _do_process_item(self, item: Any) -> None:
544568
ctx,
545569
py_version,
546570
base_sha=base_sha,
571+
benchmark_dest=benchmark_dest,
547572
)
548573

549574
# Record the container name in Supabase *before* pushing. If the DB

src/datasmith/utils/overrides.py

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
"""Read per-task operator declarations from ``formulacode_task_overrides``.
2+
3+
The overrides table holds facts about a task that cannot be derived from the
4+
repo or the PR. Two of its columns are invariant inputs, and both needed a
5+
host-side reader because the table is RLS-locked with no ``anon`` grant --
6+
neither the trial container nor the image build can query it themselves:
7+
8+
``benchmark_dest``
9+
Passed into the image build as the ``BENCHMARK_DEST`` build arg, which
10+
``docker_build_run.sh`` turns into the ``benchmark_dest_present_post_clean``
11+
breadcrumb. Input to the FATAL ``benchmark_dest_missing`` invariant.
12+
13+
``expected_n``
14+
Injected into the trial container as ``FORMULACODE_EXPECTED_N`` via the
15+
task's ``[verifier.env]`` section. Input to the ``dilution_ratio``
16+
invariant.
17+
18+
Lookup failure is never fatal. Stage 6 and stage 7 must run on a machine where
19+
the migration has not been applied, and "no override" is a legitimate state for
20+
all but a handful of tasks -- the corresponding invariants then skip, which is
21+
the correct answer rather than a degraded one.
22+
"""
23+
24+
from __future__ import annotations
25+
26+
from typing import Any
27+
28+
from datasmith.utils.core import get_logger
29+
from datasmith.utils.db import fetch_all
30+
31+
logger = get_logger("utils.overrides")
32+
33+
TABLE = "formulacode_task_overrides"
34+
35+
# The canonical task identity, per CLAUDE.md: (owner, repo, issue_number).
36+
TaskKey = tuple[str, str, int]
37+
38+
39+
_ALL_CACHE: dict[TaskKey, dict[str, Any]] | None = None
40+
41+
42+
def all_overrides(*, refresh: bool = False) -> dict[TaskKey, dict[str, Any]]:
43+
"""Every override row, read once per process.
44+
45+
The table is tiny (5 rows today, and it is hand-maintained), while its
46+
consumers sit in per-item loops -- stage 6 synthesises one PR at a time and
47+
enqueues neighbours mid-flight, so there is no single point at which the
48+
full task set is known. Caching turns what would be one round-trip per
49+
task into one per run.
50+
51+
A failed read caches ``{}`` too: if the table is absent, it will still be
52+
absent on the next call, and retrying per item would be a slow way to
53+
reach the same answer.
54+
"""
55+
global _ALL_CACHE
56+
if _ALL_CACHE is not None and not refresh:
57+
return _ALL_CACHE
58+
59+
try:
60+
rows = fetch_all(TABLE, select="owner, repo, issue_number, benchmark_dest, expected_n")
61+
except Exception:
62+
logger.warning(
63+
"Could not read %s; proceeding without overrides. The "
64+
"benchmark_dest_missing and dilution_ratio invariants will skip.",
65+
TABLE,
66+
exc_info=True,
67+
)
68+
_ALL_CACHE = {}
69+
return _ALL_CACHE
70+
71+
out: dict[TaskKey, dict[str, Any]] = {}
72+
for row in rows or []:
73+
try:
74+
out[(row["owner"], row["repo"], int(row["issue_number"]))] = row
75+
except (KeyError, TypeError, ValueError):
76+
continue
77+
_ALL_CACHE = out
78+
return out
79+
80+
81+
def fetch_overrides(tasks: list[TaskKey]) -> dict[TaskKey, dict[str, Any]]:
82+
"""Return ``{(owner, repo, issue_number): override_row}`` for *tasks*.
83+
84+
Tasks with no override row are simply absent from the mapping. A missing
85+
table, an unreachable database, or any other read failure yields ``{}``
86+
with a warning -- callers treat that identically to "no overrides exist",
87+
so a fresh checkout behaves the same as a populated one minus the extra
88+
checks.
89+
"""
90+
if not tasks:
91+
return {}
92+
93+
wanted = set(tasks)
94+
return {key: row for key, row in all_overrides().items() if key in wanted}
95+
96+
97+
def benchmark_dest_for(overrides: dict[TaskKey, dict[str, Any]], key: TaskKey) -> str:
98+
"""The declared benchmark path for *key*, or "" when undeclared.
99+
100+
Empty string is deliberate: ``docker_build_run.sh`` gates its breadcrumb on
101+
a non-empty ``$BENCHMARK_DEST``, so "" keeps the FATAL invariant skipping
102+
rather than firing against a value nobody declared.
103+
"""
104+
row = overrides.get(key) or {}
105+
return str(row.get("benchmark_dest") or "")
106+
107+
108+
def expected_n_for(overrides: dict[TaskKey, dict[str, Any]], key: TaskKey) -> int | None:
109+
"""The declared benchmark count for *key*, or None when undeclared.
110+
111+
None (not 0) is the "not judged yet" signal: the dilution invariant skips
112+
on None, and 0 would be a live comparison against a number nobody chose.
113+
"""
114+
row = overrides.get(key) or {}
115+
raw = row.get("expected_n")
116+
if raw is None:
117+
return None
118+
try:
119+
return int(raw)
120+
except (TypeError, ValueError):
121+
return None

tests/runners/test_synthesize_images.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,14 @@ def _clear_prereq_cache() -> None:
2727
"datasmith.docker.images.get_repo_image_name",
2828
return_value="formulacode/numpy-numpy:latest",
2929
)
30+
# _do_process_item looks up formulacode_task_overrides for benchmark_dest.
31+
# fetch_all resolves its own client, so without this the "mocked" runner tests
32+
# open a real Supabase connection -- passing only because a local DB happens to
33+
# be running, and failing on any machine without one.
34+
_MOCK_OVERRIDES = patch(
35+
"datasmith.runners.synthesize_images.fetch_overrides",
36+
return_value={},
37+
)
3038

3139

3240
def _mock_supabase() -> MagicMock:
@@ -77,6 +85,7 @@ async def test_docker_runs_in_thread(self) -> None:
7785
_MOCK_BUILD,
7886
_MOCK_PUSH,
7987
_MOCK_REPO_IMAGE,
88+
_MOCK_OVERRIDES,
8089
):
8190
runner = SynthesizeImagesRunner(synthesizer=synthesizer, n_concurrent=1)
8291
await runner.run([_make_item()])
@@ -112,6 +121,7 @@ async def test_handles_failure(self) -> None:
112121
_MOCK_BUILD,
113122
_MOCK_PUSH,
114123
_MOCK_REPO_IMAGE,
124+
_MOCK_OVERRIDES,
115125
):
116126
runner = SynthesizeImagesRunner(synthesizer=synthesizer, n_concurrent=1)
117127
await runner.run([_make_item()])
@@ -140,6 +150,7 @@ async def test_builds_and_pushes_on_success(self) -> None:
140150
) as mock_build,
141151
patch("datasmith.runners.synthesize_images._push_pr_image") as mock_push,
142152
_MOCK_REPO_IMAGE,
153+
_MOCK_OVERRIDES,
143154
):
144155
runner = SynthesizeImagesRunner(synthesizer=synthesizer, n_concurrent=1)
145156
await runner.run([_make_item()])
@@ -180,6 +191,7 @@ async def test_renders_and_stores_problem_statement(self) -> None:
180191
_MOCK_BUILD,
181192
_MOCK_PUSH,
182193
_MOCK_REPO_IMAGE,
194+
_MOCK_OVERRIDES,
183195
patch(
184196
"datasmith.github.render.render_problem_statement",
185197
return_value="Rendered problem text",
@@ -220,6 +232,7 @@ async def test_skips_render_without_gh(self) -> None:
220232
_MOCK_BUILD,
221233
_MOCK_PUSH,
222234
_MOCK_REPO_IMAGE,
235+
_MOCK_OVERRIDES,
223236
patch(
224237
"datasmith.github.render.render_problem_statement",
225238
) as mock_render,
@@ -267,6 +280,7 @@ async def test_render_includes_scraped_issues(self) -> None:
267280
_MOCK_BUILD,
268281
_MOCK_PUSH,
269282
_MOCK_REPO_IMAGE,
283+
_MOCK_OVERRIDES,
270284
patch(
271285
"datasmith.github.render.render_problem_statement",
272286
return_value="Rendered with issues",

0 commit comments

Comments
 (0)