Skip to content

Commit 8ea19ce

Browse files
uipreligaclaude
andcommitted
fix: reconcile the optimize-skill branch with main's CE035 and run-limit work
Integration fixes the rebase onto main exposed, in one place rather than smeared across 175 replayed commits. **The CE035 collision.** Main shipped CE035 as the workflow-output resolver (`tests/lint/workflow_outputs.py`) while this branch shipped CE035 as the split-label rule. A `# noqa` keys on the id, so one suppression would disarm both — exactly what `test_rule_ids_are_unique_across_baserules_and_test_classes` says. The unmerged one renumbers: the split-label rule is **CE060** now, in the test class, CLAUDE.md, `tasks/skills/ci-outcome.yaml`, tutorial 09 and `plan_command`'s pointer. Main's CE035 keeps its number and its file. Main's `.claude/harness-candidates.md` also reserved four numbers this branch has since implemented (CE036, CE037, CE040, CE041) plus CE042 for the sleep-loop candidate. Those reservations move to CE061–CE065, because the backlog is one of the two places CLAUDE.md tells an author to grep before claiming a number, and a reservation pointing at a shipped rule is worse than no reservation. **Main's new code under this branch's new rules.** `task_loader`'s `system_prompt_file` seam (main, #92) is a `model_copy(update={...})`, which CE048 now forbids outside `models/copy_with.py`; it becomes `copy_with(...)`, whose delegation to `model_copy` keeps the validator-skipping the two comments there describe — so the comments were corrected rather than left half-true. **Two conflicts resolved toward keeping both behaviours**, noted here because the replayed commits do not say so: `criteria/agent_judge.py` forces main's `system_prompt_file=None` / `system_prompt_mode="replace"` in the SAME `model_validate` payload as the branch's CE048 conversion (they are mutually exclusive under `validate_assignment=True`, so a sequential assignment raises), and `plan`'s per-variant `validate_run_limits` warnings (main) moved into the branch's `_report_variants`, escaped for CE050. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 7525b5c commit 8ea19ce

7 files changed

Lines changed: 24 additions & 22 deletions

File tree

.claude/harness-candidates.md

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -384,8 +384,9 @@ with the two `action.yml` items above — one considered change to the action's
384384
`timeout is None`. Caught independently by two reviewers (`bai-uipath`, `uipreliga`)
385385
on the PR, both citing the exact same arithmetic mismatch. **Not promoted in this
386386
pass**, but a stronger candidate than most entries here: `uipreliga` proposed a
387-
generic whole-tree rule (proposed as CE035, renumbered CE042 here — CE035 shipped as
388-
the workflow-outputs resolver on the published-action branch) — for every sleep-loop under
387+
generic whole-tree rule (proposed as CE035, renumbered CE065 here — CE035 shipped as
388+
the workflow-outputs resolver on the published-action branch, and CE042 as the
389+
replicate-padding seam on the optimize-skill branch) — for every sleep-loop under
389390
`src/coder_eval/agents/**`, assert its own cycle-count × interval either references a
390391
timeout-derived name or is provably below `experiments/default.yaml`'s baseline — that
391392
would catch this class of bug in ANY agent, not just this one (confirmed zero
@@ -395,7 +396,7 @@ with the two `action.yml` items above — one considered change to the action's
395396

396397
## From 2026-08-04 published-action verification review
397398

398-
- [ ] **CE041`VAR=$(… | grep …)` under `set -e` followed by an emptiness check
399+
- [ ] **CE061`VAR=$(… | grep …)` under `set -e` followed by an emptiness check
399400
is a dead diagnostic.** With `set -euo pipefail`, a pipeline whose `grep` matches
400401
nothing exits 1, so the assignment aborts the step *before* the
401402
`if [ -z "$VAR" ]; then echo "::error::…"` branch that was written to report it —
@@ -407,22 +408,22 @@ with the two `action.yml` items above — one considered change to the action's
407408
`verify-published-action.yml`; **`actionlint` + shellcheck do NOT flag it**
408409
(verified against the exact snippet), so the actionlint candidate above does not
409410
subsume this one.
410-
- [ ] **CE036 — ban the skipped-green job gate.** Fail a job-level `if:` in
411+
- [ ] **CE062 — ban the skipped-green job gate.** Fail a job-level `if:` in
411412
`.github/workflows/**` whose only discriminator is an emptiness/equality test on
412413
`needs.<job>.outputs.<key>`. A lost output on a partial "Re-run failed jobs" resolves
413414
the job to SKIPPED-**green**, so an operator sees a green re-run while nothing ran.
414415
Fixed by hand twice now: `promote` was designed around the hazard, and
415416
`publish-pypi`'s `if: needs.release.outputs.version != ''` (dead *and* dangerous — a
416417
skipped publish also skipped `promote`) was removed in the follow-up review. CE035
417418
catches the *typo* class; this catches the *shape*. Escape hatch: inline
418-
`# noqa: CE036 — <reason>` for value-driven gates that cannot strand a release.
419-
- [ ] **CE037`if: failure()` is wrong in a job containing a `continue-on-error`
419+
`# noqa: CE062 — <reason>` for value-driven gates that cannot strand a release.
420+
- [ ] **CE063`if: failure()` is wrong in a job containing a `continue-on-error`
420421
step.** Require `always()` (or a reference to the tolerated step's
421422
`steps.<id>.outcome`) on diagnostic/upload steps in such a job. Fixed by hand in
422423
`verify-published-action.yml`: the run dir was discarded in exactly the tolerated-red
423424
case the gate is designed around, because a tolerated red leaves the job green and
424425
`failure()` never fires. Pure YAML shape check, ~30 lines.
425-
- [ ] **CE040 — cap inline `run:` bodies; oversized decision logic belongs in
426+
- [ ] **CE064 — cap inline `run:` bodies; oversized decision logic belongs in
426427
`.github/scripts/`.** `verify-published-action.yml`'s parity step (~70 lines, 7
427428
decision points) and its e2e gate (~66 lines, switching from bash to a `python3`
428429
heredoc mid-step) are 10-20-branch units invisible to `make check`, `make lint`,

CLAUDE.md

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

docs/tutorials/09-optimizing-a-skill-body.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ dataset:
5858

5959
Label every row or none. A *partly* labelled dataset is the one genuinely bad state: `--split`
6060
keeps the rows that match and silently drops the unlabelled ones, so the run succeeds and every
61-
metric is computed over a smaller suite than the file suggests. `CE035` fails the build on it.
61+
metric is computed over a smaller suite than the file suggests. `CE060` fails the build on it.
6262

6363
### One fixture, and the variation lives in the prompt
6464

src/coder_eval/cli/plan_command.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ def _preview_dataset(
9090
# already in hand.
9191
rows = load_dataset_rows(task.dataset, task_file.parent)
9292
# `row_split_label` is the runtime's single definition of "labelled" (select_rows and
93-
# CE035 call it too) — do not re-derive the rule here.
93+
# CE060 call it too) — do not re-derive the rule here.
9494
labels = [row_split_label(r, task.dataset.split_field) for r in rows]
9595
labelled = [x for x in labels if x is not None]
9696

src/coder_eval/orchestration/task_loader.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
TaskDefinition,
2222
TemplateDirSource,
2323
TemplateSource,
24+
copy_with,
2425
)
2526

2627

@@ -289,10 +290,10 @@ def resolve_agent_system_prompt[T: AgentConfig | BaseAgentConfig | None](agent_c
289290
if not prompt_path.exists():
290291
raise FileNotFoundError(f"system_prompt_file not found: {prompt_path}")
291292
# A whitespace-only file is no prompt at all — mirror the normalization
292-
# _blank_prompt_is_no_prompt applies to inline prompts (model_copy skips
293-
# validators, so this seam has to apply it itself).
293+
# _blank_prompt_is_no_prompt applies to inline prompts (copy_with delegates to
294+
# model_copy, which skips validators, so this seam has to apply it itself).
294295
content = prompt_path.read_text(encoding="utf-8").strip() or None
295-
# ...which means model_copy also skips check_replace_mode_has_prompt, so a
296+
# ...which means the copy also skips check_replace_mode_has_prompt, so a
296297
# blank file under `replace` would reach the agent as (replace, no prompt)
297298
# and silently downgrade to the append preset at runtime. Reject it here
298299
# instead: the file is the only prompt the config had, and the docs promise
@@ -302,7 +303,7 @@ def resolve_agent_system_prompt[T: AgentConfig | BaseAgentConfig | None](agent_c
302303
f"system_prompt_file {prompt_path} is empty; system_prompt_mode='replace' requires a "
303304
+ "prompt to replace the Claude Code default with"
304305
)
305-
return agent_config.model_copy(update={"system_prompt": content, "system_prompt_file": None})
306+
return copy_with(agent_config, system_prompt=content, system_prompt_file=None)
306307

307308

308309
def resolve_system_prompt_files(task: TaskDefinition, base_dir: Path) -> TaskDefinition:

tasks/skills/ci-outcome.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ run_limits:
111111
# instruction in the body observable.
112112
#
113113
# The fixture lives under `templates/`, NOT beside this file, and that is deliberate:
114-
# anything matching `tasks/**/*.yaml` is loaded as a real TaskDefinition by CE034, CE035,
114+
# anything matching `tasks/**/*.yaml` is loaded as a real TaskDefinition by CE034, CE060,
115115
# test_tags and test_yaml_migration — and this fixture must contain an experiment file,
116116
# which is not a valid task. A fixture placed beside its suite would break all four.
117117
sandbox:

tests/test_custom_lint.py

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3027,7 +3027,7 @@ def test_outcome_rows_are_all_positive(self):
30273027
assert rows, "the outcome template ships no rows"
30283028
# `row_split_label`, not `r.get("split")`: truthiness would report a legitimate
30293029
# `"split": 0` as unlabelled, which is a SECOND definition of "labelled" competing
3030-
# with the runtime's. CE035 exists precisely to have one.
3030+
# with the runtime's. CE060 exists precisely to have one.
30313031
assert all(row_split_label(r, "split") is not None for r in rows), (
30323032
"every template row must carry a `split` — a PARTLY labelled dataset is the one bad "
30333033
"state: --split keeps the matching rows and drops the unlabelled ones, shrinking the "
@@ -3278,7 +3278,7 @@ def test_activation_rows_split_both_polarities_both_sides(self):
32783278
]
32793279
# `row_split_label`, not `r.get("split")`: truthiness would report a legitimate
32803280
# `"split": 0` as unlabelled, which is a SECOND definition of "labelled" competing
3281-
# with the runtime's. CE035 exists precisely to have one.
3281+
# with the runtime's. CE060 exists precisely to have one.
32823282
assert all(row_split_label(r, "split") is not None for r in rows), (
32833283
"every template row must carry a `split` — a PARTLY labelled dataset is the one "
32843284
"bad state: --split keeps the matching rows and drops the unlabelled ones, "
@@ -6469,8 +6469,8 @@ def test_finding_reports_the_line_of_the_offending_reference(self, tmp_path: Pat
64696469

64706470

64716471
@pytest.mark.lint
6472-
class TestCE035SplitLabelsAllOrNothing:
6473-
"""CE035 — a dataset's split field must be on every row or on none, never on some.
6472+
class TestCE060SplitLabelsAllOrNothing:
6473+
"""CE060 — a dataset's split field must be on every row or on none, never on some.
64746474

64756475
`optimize-skill` calls a partly-labelled dataset "the dangerous state, because it does
64766476
not look like one", and it is right: ``--split`` keeps the rows whose label matches and
@@ -6765,9 +6765,9 @@ def test_ce036_exemption_list_matches_claude_md(self):
67656765

67666766

67676767
def _dataset_task(rows: list[dict], *, prompt: str = "${row.id}", criteria=None, split_field: str = "split"):
6768-
"""A minimal dataset-backed task over inline rows, for the CE035/CE036 fixtures.
6768+
"""A minimal dataset-backed task over inline rows, for the CE060/CE036 fixtures.
67696769

6770-
One builder for both rules: CE035 needs varying rows, CE036 varying prompts AND
6770+
One builder for both rules: CE060 needs varying rows, CE036 varying prompts AND
67716771
criteria, and a per-class copy would fork the moment either grew a parameter.
67726772
Inline ``rows`` deliberately — no JSONL file, so the fixtures never touch disk.
67736773
"""
@@ -8940,7 +8940,7 @@ class TestCE052TemplateTasksLoad:
89408940
"""CE052 — every task YAML under `templates/` must load through the real `load_task`.
89418941

89428942
A `@pytest.mark.lint` class rather than a `BaseRule`: it reasons over YAML trees and needs the
8943-
loader itself, not one `.py` AST at a time — the same shape as CE035/CE036.
8943+
loader itself, not one `.py` AST at a time — the same shape as CE060/CE036.
89448944

89458945
**What it caught.** `templates/ci-outcome-fixture/evals/activation.yaml` declared
89468946
`suite_thresholds: {recall.yes: 0.7}` with no `dataset:` block, which

0 commit comments

Comments
 (0)