fix(batches): resolve batch ids beyond the 100-batch cap (#825)

[frankbria]

Summary

Fixes #825 — the same class of truncation bug as #743, applied to the Batch entity.

The four cf work batch status/stop/resume/follow commands resolved a partial
batch id in Python over conductor.list_batches(workspace, limit=100). Any
batch beyond position 100 (the oldest ones, since list_batches orders
newest-first) was unreachable by partial id — the user would get "No batch
found" even though the batch existed.

What changed

• codeframe/core/conductor.py — new find_batch_by_prefix(workspace, prefix):
  SQL LIKE 'prefix%' ESCAPE '\' with no LIMIT, mirroring tasks.find_by_prefix.
  LIKE metachars (\, %, _) in the user-typed prefix are escaped before
  the query so a stray wildcard cannot over-match.

• codeframe/cli/app.py — all four resolution sites replaced:
  list_batches(workspace, limit=100) + Python startswith filter →
  find_batch_by_prefix(workspace, batch_id).

• tests/core/test_batch_truncation.py — regression suite (6 tests),
  mirrors test_tasks_truncation.py:

  • resolves batch at position 101+ (150-batch fixture)
  • exact full UUID resolves
  • no match → empty list
  • ambiguous prefix → all matches returned
  • LIKE metachar prefix (%, _) → empty (no over-match)
  • backslash prefix → empty (no crash)

Acceptance Criteria

• cf work batch status <partial-id> resolves batches beyond position 100
• cf work batch stop <partial-id> resolves batches beyond position 100
• cf work batch resume <partial-id> resolves batches beyond position 100
• cf work batch follow <partial-id> resolves batches beyond position 100
• LIKE metachar escaping prevents over-matching

Test Plan

• 6 new regression tests — all passing
• Existing tests unchanged: test_tasks_truncation.py (3 tests), test_batch_run_connection_safety.py (13 tests) — all green
• ruff: clean
• mypy: clean
• Pre-commit hooks: passed

Known Limitations / Intentionally Deferred

• list_batches default limit of 20 (and the limit=100 used in the old
  resolution path) is unchanged — find_batch_by_prefix is the resolution
  path only. Unbounded batch listing (analogous to list_tasks(limit=None))
  is not addressed here; that is a separate concern if needed.

Implementation Notes

The fix is intentionally minimal — same shape as the tasks fix in #824 so the
pattern is easy to verify. No behavior changes beyond making batches 101+
addressable by partial id.

Closes #825

[github-actions]

Claude finished @frankbria's task in 7m 47s —— View job

---

I'll analyze this and get back to you.

[claude]

Review

This is the first review on this PR (no prior reviews/comments to reconcile with — just the automated "Claude Code is working" placeholder). Solid, narrowly-scoped fix that mirrors the accepted tasks.find_by_prefix pattern from #743/#824 almost line-for-line, which is the right call for reviewability.

Verified:

• find_batch_by_prefix (codeframe/core/conductor.py:830) correctly drops the LIMIT, filters by workspace_id, and escapes LIKE metachars (\, %, _) in the same order as tasks.find_by_prefix — no over-match risk.
• id is TEXT PRIMARY KEY on batch_runs (codeframe/core/workspace.py:289), so the prefix scan has an index to use, consistent with the tasks table.
• All four CLI call sites (batch_status, batch_stop, batch_resume, the _update_progress/batch_follow path) were updated; the unrelated list_batches(workspace, limit=10) at app.py:4081 (recent-batches listing, no id given) was correctly left alone — out of scope.
• Test suite mirrors test_tasks_truncation.py well: beyond-100 resolution, exact-id, no-match, ambiguous, and both LIKE-metachar and backslash escaping cases are covered.

Minor, non-blocking observations:

1. Ambiguity handling is inconsistent across the four call sites, and this PR slightly raises the odds of hitting it. batch_status and batch_stop (and the batch_follow progress-tracking path) silently take matching[0] when more than one batch matches a prefix — no len(matching) > 1 check like the task-resolution call sites have (e.g. app.py:2484). batch_resume does print a "Multiple batches match" warning but then appears to proceed anyway rather than exiting (worth double-checking that path doesn't act on the wrong batch). This isn't introduced by this PR — the old list_batches(limit=100) + startswith code had the exact same gap — but removing the cap means prefix collisions are now checked against the entire batch history instead of just the newest 100, which does increase collision odds for short user-typed prefixes over a project's lifetime. Given batch_stop/batch_resume are mutating operations, silently acting on matching[0] (the newest match) on an ambiguous prefix is a sharper edge now than it was before. Worth a follow-up issue if not this PR.

2. SQL LIKE is case-insensitive by default in SQLite (for ASCII), while the old str.startswith was case-sensitive. A user typing AB12 would now match a batch id starting ab12... where it wouldn't have before. Same quirk already exists in tasks.find_by_prefix, so this is consistent with precedent rather than a new problem — flagging only because it's a real (if narrow) behavioral change from the code being replaced.

3. Test suite uses the private _save_batch helper directly to seed fixtures rather than going through a public "create batch" API — reasonable given there's no lightweight way to create a BatchRun without spawning real workers, and the docstring calls this out explicitly.

Nothing here blocks merging — the core fix is correct, well-tested, and consistent with the established #743 pattern. Item 1 is the only one I'd consider worth a fast follow-up given it affects mutating commands (stop/resume).

[frankbria]

Cross-family review — opencode (zai/glm-5.2)

Adversarial Review — PR #872

I traced all four call sites and confirmed the wiring against tasks.find_by_prefix (the accepted analog from #743/#824) plus the task-side CLI resolution pattern.

Findings

Critical — codeframe/cli/app.py:4171 (batch_stop)
Mutating command resolves batch = matching[0] with no len(matching) > 1 check; widening resolution from a 100-row window to the entire batch_runs history materially raises the probability that a short prefix matches ≥2 batches, and the CLI then silently cancels whichever happens to sort first by started_at DESC.
Failure scenario: 250-batch workspace, two historical batches share prefix a1b2 (4-hex-char collisions become likely at this scale). User runs cf work batch stop a1b2 intending the older of two running batches; the CLI resolves to the newer match and stops an unrelated running batch — no error, no warning, graceful-stop semantics mean the wrong work is terminated. The author's own docstring says the function returns all matches "so callers can report ambiguity"; the one mutating caller does not.

Major — codeframe/cli/app.py:4239-4245 (batch_resume)
This site detects ambiguity but only emits a yellow warning and proceeds with matching[0]. With --force it re-runs every task on whatever batch sorted first. Pre-existing behavior, but this PR is what makes the warn-and-proceed path meaningfully more likely to fire, so it belongs in scope.
Failure scenario: cf work batch resume abc1 --force prints a warning that scrolls past; the wrong batch's completed tasks are all re-executed.

Major — codeframe/cli/app.py:4027 (batch_status) and :~4432 (batch_follow)
Read-only callers also silently take matching[0] with no ambiguity signal, asymmetric with batch_resume's warning and with the docstring's stated contract.
Failure scenario: user decides whether to intervene based on status of the wrong batch; follow tails the wrong batch's event stream and the user waits on a batch that already finished.

Major — tests/core/test_batch_truncation.py
No CLI-surface test exercises ambiguity handling for any of the four commands. Every task-side site (app.py:2194, 2309, 2478, 2617, …) does if len(matching) > 1: error + raise typer.Exit(1); the batch sites diverge and nothing in the suite catches the divergence. The new tests prove the function is correct; they don't prove the wiring is safe.

Suggestion — codeframe/core/conductor.py:830-867
No LIMIT, and test_ambiguous_prefix_returns_multiple proves an empty prefix loads every batch_runs row as a BatchRun. On a long-lived workspace a short or empty prefix materializes the full history. Callers only need to know 0 / 1 / >1, so a modest cap (e.g. LIMIT 1000) preserves correctness without the unbounded read.

Suggestion — codeframe/core/conductor.py:862
ORDER BY started_at DESC with no COALESCE/null handling; if any row can have NULL started_at, SQLite sorts NULLs first and matching[0] silently becomes that row. Worth confirming against the schema's nullability for started_at on PENDING batches.

Nitpick — tests/core/test_batch_truncation.py:46
Class name typo: TestFindBatchByPrefixResolvesBeyon100 (missing "d"). Objective, harmless.

Notes that are not findings

• LIKE escape order (\ → % → _) is correct and matches tasks.find_by_prefix:248.
• Parameterized query — no injection.
• Beyond-100 resolution (AC1) and LIKE-metachar neutralization (AC2) are both genuinely satisfied by the new function and covered by tests.

Verdict

REQUEST_CHANGES — the conductor.find_batch_by_prefix implementation is correct and ship-worthy, but the PR changes the collision surface for four callers without updating their ambiguity handling. The minimum acceptable fix: add the len(matching) > 1 → error + typer.Exit(1) guard (matching the task-side pattern at app.py:2194) to at least the two mutating commands (batch_stop, batch_resume), and ideally all four for consistency.

[github-actions]

Claude finished @frankbria's task in 3m 38s —— View job

---

GLM precision review — PR #872 (re-review after 8b2a7d9)

Bug-hunting pass on the current diff (both commits) — logic errors, injection, data loss, races, contract breaks only. No style/coverage commentary.

• Pull PR context + full diff (gh pr view, git diff origin/main...HEAD)
• Read prior review comments — do not repeat already-posted findings
• Verify find_batch_by_prefix against _row_to_batch / list_batches / schema
• Verify all 4 CLI resolution sites + ambiguity guards
• Check task-side analog (tasks.find_by_prefix) for parity
• Report confirmed defects (inline) + summary

✅ No new concrete defects found

Verified against the checked-out repo (not the diff alone):

Area	Result
find_batch_by_prefix SELECT column order (conductor.py:855)	Matches _row_to_batch (2441), list_batches (797), _save_batch (2410) — all 17 columns in the same positions ✓
LIKE escape order \→%→_ (conductor.py:849)	Correct; backslash-first avoids double-escaping the escapes it adds. Mirrors accepted tasks.find_by_prefix:248 ✓
Injection / param binding (conductor.py:860-863)	Parameterized (workspace.id, f"{escaped}%"); prefix never string-interpolated into SQL ✓
Connection lifecycle (conductor.py:850-867)	Closed in finally, no leak ✓
ORDER BY started_at DESC NULL concern	started_at is TEXT NOT NULL on batch_runs (workspace.py:296, 518) — concern moot ✓
Ambiguity guards, all 4 sites (app.py:4027, 4177, 4251, 4449)	Each does len(matching) > 1 → "Multiple batches match" + typer.Exit(1), mirroring the task-side pattern at app.py:2484. grep confirms no unguarded matching[0] remains for batches; the old list_batches(workspace, limit=100) + startswith filter is fully gone ✓
b.status.value in error output	BatchStatus(str, Enum) (conductor.py:526) — .value is valid ✓
Test suite validity	_save_batch (conductor.py:2407) and workspace.repo_path both exist; the parametrized CLI ambiguity test seeds a real colliding pair and asserts exit_code == 1 ✓

Already-posted findings — not repeated: ambiguity handling on stop/resume/status/follow (raised by both prior reviews) is fully resolved by 8b2a7d9. SQLite ASCII case-insensitivity of LIKE and the unbounded (no-LIMIT) read were both flagged earlier; both are consistent with the accepted tasks.find_by_prefix precedent and are not new defects in this PR.

The implementation is a faithful, correct mirror of the accepted #743/#824 task-resolution pattern, with the collision-surface gap from the prior cross-family review now closed on all four mutating + read-only commands.

---

[claude]

Follow-up review — second commit (8b2a7d9)

Re-reviewing after the cross-family (opencode) adversarial review and the fix commit that followed it.

Confirmed: the REQUEST_CHANGES-blocking findings are resolved.

• All four call sites (batch_status:4021, batch_stop:4171, batch_resume:4245, _update_progress/batch_follow:4443) now have if len(matching) > 1: ... raise typer.Exit(1) before matching[0] is used — including the two that previously had no guard at all (batch_status, batch_stop, batch_follow) and batch_resume, which previously warned-then-proceeded and now errors+exits like the rest. This is the correct fix and it's minimal.
• These typer.Exit(1) guards sit inside try blocks that only catch FileNotFoundError/ValueError (checked all four), so the exit isn't accidentally swallowed by a broad except Exception.
• New TestCliAmbiguousPrefixIsRejected parametrizes over all four subcommands via CliRunner, seeding two batches with a colliding dupe prefix — this is exactly the CLI-wiring test the prior review said was missing, and it closes the gap between "the function is correct" and "the callers use it safely."
• Class-name typo (Beyon100 → Beyond100) is fixed.

One suggestion from the opencode review is moot, not just unaddressed: the concern about ORDER BY started_at DESC and NULL ordering — batch_runs.started_at is TEXT NOT NULL in the schema (codeframe/core/workspace.py), so there's no NULL-first surprise to guard against. Worth closing that thread out explicitly rather than leaving it open.

Still open, still non-blocking (same suggestion as before, not re-addressed):

• find_batch_by_prefix still has no LIMIT. Now that every caller correctly branches on 0 / 1 / >1, a modest cap (e.g. 1000) would still be a reasonable defensive follow-up for a very long-lived workspace where an empty/short prefix currently materializes the entire batch_runs history — but this doesn't affect correctness of this PR and is fine to defer.
• Minor cosmetic divergence from the task-side pattern (app.py:2194): the batch ambiguity messages truncate silently at 5 matches with no "+N more" indicator and don't include the tasks-side "Please provide a more specific ID." hint. Not worth blocking on.

Nothing else to add beyond what's already been covered across the two rounds of review — the core fix, the escaping, and the test coverage were solid from the first commit; the ambiguity-handling gap the second commit closes was the one thing worth blocking on, and it's closed correctly with test coverage to prove the wiring, not just the function. No remaining objections from me.

[frankbria]

Merge triage summary (final gate)

Cross-family review: opencode (zai/glm-5.2) — entered on an existing PR without a prior Phase-7 gate, so the post-PR cross-family pass was run now and posted above. Verdict was REQUEST_CHANGES (Critical on batch_stop); addressed in 8b2a7d9. GLM CI re-review + Claude follow-up both confirm the blocking findings are resolved with no new defects.

Findings triage:

Finding	Severity	Resolution
batch_stop silently acts on matching[0] on ambiguous prefix (mutating; PR widened collision surface)	Critical	Fixed — len>1 → error + typer.Exit(1) guard
batch_resume warned then proceeded	Major	Fixed — now errors + exits
batch_status/batch_follow silent matching[0]	Major	Fixed — same guard, all 4 sites consistent
No CLI-surface ambiguity test	Major	Fixed — parametrized CliRunner test over all 4 commands
Class-name typo Beyon100	Nitpick	Fixed
No LIMIT on find_batch_by_prefix	Suggestion	Deferred — consistent with accepted tasks.find_by_prefix; real prefixes are non-empty UUIDs (empty-prefix full read only occurs in a test). Non-blocking.
LIKE ASCII case-insensitivity	Suggestion	Deferred — same precedent as tasks.find_by_prefix; not new to this PR.
Ambiguity msg lacks "+N more"/"more specific id" hint vs task-side	Nitpick	Deferred — cosmetic; reviewers agreed not worth blocking.
NULL started_at ordering	Suggestion	Moot — started_at TEXT NOT NULL; same ordering as list_batches.

Demo (Phase 11) — all acceptance criteria verified against the real CLI on a 156-batch workspace with 4 named targets seeded at positions 153–156 (beyond the old 100-cap): status/stop/resume/follow each resolved their beyond-100 target; status % returned "No batch found" (metachar escaped, no over-match); stop dupe exited 1 with "Multiple batches match".

All CI green on 8b2a7d9. Merging.