fix(runner): stream --ids sources and cap selection to the pinned set

[SebTardif]

What Problem This Solves

Fixes an issue where clawscan benchmark --ids could exhaust memory. readBenchmarkIDSource did an unbounded io.ReadAll on HTTP sources (and os.ReadFile on local files). Follow-up to #45, which @steipete closed: a 256 KiB HTTP-only cap would reject the documented full SkillTrustBench ID list (5,520 cases, about 1.3 MiB of canonical JSONL) and left files unbounded.

The first head still kept every parsed id in memory up to the 5,520-count cap. A hostile source can send 5,520 unique records that are each far larger than a real case id. This tip also caps one extracted id at 256 bytes and retained id text at 256 KiB. Those are not JSONL file-size limits, so the documented 5,520-id / 1.3 MiB SkillTrustBench stream still parses.

Why This Change Was Made

Stream file and HTTP --ids sources through the same line parser. Bound the selection by case count (5,520 unique IDs, the pinned SkillTrustBench full set), one-id length (256 bytes), and retained id text (256 KiB). Do not copy the 256 KiB plugin-manifest limit onto the JSONL stream.

User Impact

A hostile or accidental multi-gigabyte --ids URL no longer has to be fully buffered. A valid full-set JSONL list still loads. More than 5,520 unique IDs, a single id longer than 256 bytes, or more than 256 KiB of retained id text is rejected before the benchmark run starts.

Evidence

Terminal output from the public CLI built at 6baa16f. Selection load rejects the new retained-id bounds and still accepts a 299600-byte JSONL file (larger than 256 KiB):

$ /tmp/clawscan-47 benchmark SkillTrustBench --ids /tmp/clawscan-huge-id.txt --scanner clawscan-static --output /tmp/clawscan-ids-out.json
--ids source /tmp/clawscan-huge-id.txt line 1 exceeds the 256-byte benchmark id limit
exit status 1

$ /tmp/clawscan-47 benchmark SkillTrustBench --ids /tmp/clawscan-agg-ids.txt --scanner clawscan-static --output /tmp/clawscan-ids-out.json
--ids source /tmp/clawscan-agg-ids.txt exceeds the 262144-byte retained-id budget
exit status 1

$ /tmp/clawscan-47 benchmark SkillTrustBench --ids /tmp/clawscan-too-many-ids.txt --scanner clawscan-static --output /tmp/clawscan-ids-out.json
--ids source /tmp/clawscan-too-many-ids.txt exceeds the 5520-id SkillTrustBench selection limit
exit status 1

A 400-row JSONL fixture (299600 bytes) loads from a file. The CLI then looks up selected ids in SkillTrustBench, which proves selection finished and the 256 KiB stream cap is not applied:

$ wc -c /tmp/clawscan-large-ids.jsonl
299600 /tmp/clawscan-large-ids.jsonl
$ /tmp/clawscan-47 benchmark SkillTrustBench --ids /tmp/clawscan-large-ids.jsonl --scanner clawscan-static --output /tmp/clawscan-large-out.json
--ids requested benchmark id case_00046, but it is missing from SkillTrustBench split benchmark
exit status 1

The same 400-id selection also loads from HTTP (loaded count=400, matching sha256 835384f6bb8da14b7a7db4cdf6fc1c844331eaa3a05244064f9351b58d69e1bb).

make docs-site on this tree: Built 7 docs page(s) in dist/docs-site. Generated dist/ was not committed.

Real behavior proof

• Behavior or issue addressed: --ids no longer does an unbounded full-body read. One extracted id is capped at 256 bytes and retained id text at 256 KiB. A valid JSONL list larger than 256 KiB still loads.

• Real environment tested: macOS, Go toolchain, branch fix/ids-stream-semantic-bound at 6baa16f, public CLI binary /tmp/clawscan-47 built from that tree.

• Exact steps or command run after this patch:

  python3 -c "from pathlib import Path; Path('/tmp/clawscan-huge-id.txt').write_text('a'*257+'\n')"
  /tmp/clawscan-47 benchmark SkillTrustBench --ids /tmp/clawscan-huge-id.txt --scanner clawscan-static --output /tmp/clawscan-ids-out.json
  /tmp/clawscan-47 benchmark SkillTrustBench --ids /tmp/clawscan-agg-ids.txt --scanner clawscan-static --output /tmp/clawscan-ids-out.json
  /tmp/clawscan-47 benchmark SkillTrustBench --ids /tmp/clawscan-too-many-ids.txt --scanner clawscan-static --output /tmp/clawscan-ids-out.json
  /tmp/clawscan-47 benchmark SkillTrustBench --ids /tmp/clawscan-large-ids.jsonl --scanner clawscan-static --output /tmp/clawscan-large-out.json

• Evidence after fix: terminal output from the public CLI on 6baa16f:

  $ /tmp/clawscan-47 benchmark SkillTrustBench --ids /tmp/clawscan-huge-id.txt --scanner clawscan-static --output /tmp/clawscan-ids-out.json
  --ids source /tmp/clawscan-huge-id.txt line 1 exceeds the 256-byte benchmark id limit
  exit status 1
  $ /tmp/clawscan-47 benchmark SkillTrustBench --ids /tmp/clawscan-agg-ids.txt --scanner clawscan-static --output /tmp/clawscan-ids-out.json
  --ids source /tmp/clawscan-agg-ids.txt exceeds the 262144-byte retained-id budget
  exit status 1
  $ /tmp/clawscan-47 benchmark SkillTrustBench --ids /tmp/clawscan-too-many-ids.txt --scanner clawscan-static --output /tmp/clawscan-ids-out.json
  --ids source /tmp/clawscan-too-many-ids.txt exceeds the 5520-id SkillTrustBench selection limit
  exit status 1
  $ wc -c /tmp/clawscan-large-ids.jsonl
  299600 /tmp/clawscan-large-ids.jsonl
  $ /tmp/clawscan-47 benchmark SkillTrustBench --ids /tmp/clawscan-large-ids.jsonl --scanner clawscan-static --output /tmp/clawscan-large-out.json
  --ids requested benchmark id case_00046, but it is missing from SkillTrustBench split benchmark
  exit status 1

• Observed result after fix: The CLI exits 1 at selection load for a 257-byte id, for 2000 retained ids over the 262144-byte budget, and for 5521 ids. A 299600-byte JSONL source is accepted and the next error is a missing SkillTrustBench row, not a stream-size reject. Hugging Face row fetch is not started on the reject paths.

• What was not tested: A live Hugging Face --ids URL for the full 5,520-row JSONL (about 1.3 MiB). Coverage for that size class is the 400-row file and HTTP fixture (299600 bytes).

Summary

• What changed: stream --ids file and HTTP sources; cap unique IDs at the pinned SkillTrustBench set size; cap one id at 256 bytes and retained id text at 256 KiB.
• Why: unbounded ReadAll/ReadFile can exhaust memory; a 256 KiB HTTP file-size cap rejects valid documented input; a count-only cap still lets 5,520 huge records stay in memory.

Scope

• CLI behavior
• Judge/profile/benchmark behavior

Security / Trust Impact

• Security/trust impact explained

Unbounded remote --ids bodies are no longer fully buffered. Limits are the SkillTrustBench case count, a 256-byte per-id cap, and a 256 KiB retained-id-text budget, documented in docs/benchmarks.md.

Verification

• Focused scanner/benchmark/manual proof: public CLI reject of a 257-byte id, a 262144-byte retained-id budget overflow, and 5521 IDs; public CLI accept of a 299600-byte JSONL source (above)
• Docs site proof (make docs-site) or N/A: Built 7 docs page(s) in dist/docs-site (generated dist/ not committed)

Ref #45

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

[clawsweeper]

Codex review: needs changes before merge. Reviewed August 31, 2026, 3:58 AM ET / 07:58 UTC.

ClawSweeper review

What this changes

The PR streams SkillTrustBench ID selections from files and URLs, limits retained IDs and text, adds boundary tests, and updates benchmark guidance.

Merge readiness

⚠️ Ready for maintainer review - 3 items remain

Keep open: current main still fully buffers local and HTTP --ids sources, so this remains a useful bounded fix. The implementation and supplied terminal proof are sound; one low-priority documentation gap remains.

Priority: P2
Reviewed head: 6baa16f5fe70e10ca1f69925ae1fe25d034a2b9a

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	The core fix is focused and well-proven, with only a small operator-documentation correction remaining.
Proof confidence	🦞 diamond lobster (5/6)	Sufficient (terminal): The supplied current-head terminal transcript exercises the public CLI's new rejection paths and confirms a 299,600-byte JSONL source reaches benchmark lookup rather than a stream-size rejection; the shared parser and focused test cover the corresponding HTTP path.
Patch quality	🐚 platinum hermit (4/6)	1 actionable review finding remain.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (terminal): The supplied current-head terminal transcript exercises the public CLI's new rejection paths and confirms a 299,600-byte JSONL source reaches benchmark lookup rather than a stream-size rejection; the shared parser and focused test cover the corresponding HTTP path.
Evidence reviewed	5 items	Current main still has the memory-risk path: Current main reads the entire HTTP body or local file into memory before parsing, so the central problem is not already implemented or released. Introduced implementation bounds retained data: The branch passes one reader to the parser and rejects an oversized extracted ID or cumulative retained ID text before inserting it into the retained selection. Boundary coverage exercises file and HTTP selection loading: Tests cover a JSONL source larger than 256 KiB through both file and HTTP paths, plus per-ID, aggregate-text, and count-limit rejection.
Findings	1 actionable finding	[P3] Document the per-ID and retained-text limits
Security	None	None.

How this fits together

The benchmark command reads an optional local or HTTP ID selection before looking up SkillTrustBench rows and running scanners. That selection determines which benchmark cases enter artifact generation and scanner execution.

flowchart LR
A[ID file or URL] --> B[Selection loader]
B --> C[Line parser]
C --> D[Count and size checks]
D --> E[Selected benchmark IDs]
E --> F[Benchmark row lookup]
F --> G[Scanner runs and artifact]

Loading

Before merge

• Document the per-ID and retained-text limits (P3) - The loader now rejects IDs over 256 bytes and selections retaining over 256 KiB, but this changed guide only names the 5,520-ID limit. Document all three limits and clarify that they do not cap the streamed source size; synchronize the README and CLI skill so operators can predict valid input.
• Resolve merge risk (P1) - The new 256-byte per-ID and 256-KiB retained-text limits are enforced but only partially documented, so operators may encounter an unexpected validation failure.
• Complete next step (P2) - A narrow documentation-only repair can resolve the remaining P3 finding without changing the bounded loader behavior.

Findings

• [P3] Document the per-ID and retained-text limits — docs/benchmarks.md:15-17

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Production versus test delta	production +39/-12, tests +85	The bounded loader change is accompanied by focused coverage for both transports and all three new limits.

Root-cause cluster

Relationship: canonical
Canonical: #47
Summary: This PR is the viable follow-up for the same unbounded --ids loading concern after the earlier HTTP-only approach was closed unmerged.

Members:

• superseded: fix(runner): cap HTTP --ids response bodies #45 - It proposed an HTTP-only size cap that would reject the documented full JSONL selection; this PR replaces that approach with streaming and retained-data bounds.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge-risk options

Maintainer options:

1. Decide the mitigation before merge
   Keep the streaming implementation and document all three selection limits, explicitly distinguishing retained-ID limits from a streamed source-file size limit.
2. Pause or close
   Do not merge this PR until maintainers decide whether the risk is worth taking.

Technical review

Best possible solution:

Keep the streaming implementation and document all three selection limits, explicitly distinguishing retained-ID limits from a streamed source-file size limit.

Do we have a high-confidence way to reproduce the issue?

Yes, source-reproducible: current main's LoadBenchmarkIDSelection fully reads the local file or HTTP response before parsing, and the supplied current-head terminal transcript exercises the after-fix CLI behavior.

Is this the best way to solve the issue?

Yes, the shared streaming reader keeps file and HTTP behavior aligned while the retained-data limits bound the selection without imposing a source-file-size cap.

Full review comments:

• [P3] Document the per-ID and retained-text limits — docs/benchmarks.md:15-17
  The loader now rejects IDs over 256 bytes and selections retaining over 256 KiB, but this changed guide only names the 5,520-ID limit. Document all three limits and clarify that they do not cap the streamed source size; synchronize the README and CLI skill so operators can predict valid input.
  Confidence: 0.99

Overall correctness: patch is correct
Overall confidence: 0.95

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 9218ebfaff8c.

Labels

Label changes:

• remove merge-risk: 🚨 compatibility: Current PR review selected no merge-risk labels.

Label justifications:

• P2: This is a bounded public-CLI reliability and resource-safety fix with limited benchmark-only blast radius.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The supplied current-head terminal transcript exercises the public CLI's new rejection paths and confirms a 299,600-byte JSONL source reaches benchmark lookup rather than a stream-size rejection; the shared parser and focused test cover the corresponding HTTP path.
• proof: sufficient: Contributor real behavior proof is sufficient. The supplied current-head terminal transcript exercises the public CLI's new rejection paths and confirms a 299,600-byte JSONL source reaches benchmark lookup rather than a stream-size rejection; the shared parser and focused test cover the corresponding HTTP path.

Evidence

Acceptance criteria:

• [P1] go test -count=1 ./...
• [P1] go vet ./...
• [P1] make docs-site.

What I checked:

• Current main still has the memory-risk path: Current main reads the entire HTTP body or local file into memory before parsing, so the central problem is not already implemented or released. (internal/runner/benchmark.go:315, 9218ebfaff8c)
• Introduced implementation bounds retained data: The branch passes one reader to the parser and rejects an oversized extracted ID or cumulative retained ID text before inserting it into the retained selection. (internal/runner/benchmark.go:365, 6baa16f5fe70)
• Boundary coverage exercises file and HTTP selection loading: Tests cover a JSONL source larger than 256 KiB through both file and HTTP paths, plus per-ID, aggregate-text, and count-limit rejection. (internal/runner/runner_test.go:804, 6baa16f5fe70)
• User-facing guidance is incomplete: The changed benchmark guide documents streaming and the 5,520-ID cap, but omits the enforced 256-byte per-ID and 256-KiB retained-text limits. (docs/benchmarks.md:15, 6baa16f5fe70)
• Feature-history routing: Current-main history shows Patrick Erichsen as the most frequent contributor to the benchmark implementation, with recent runner work also by Jesse Merhi and SebTardif. (internal/runner/benchmark.go:310, 58f60aca5b18)

Likely related people:

• Patrick Erichsen: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• Jesse Merhi: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• SebTardif: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

• Document the 256-byte per-ID and 256-KiB retained-text limits alongside the 5,520-ID cap in the public benchmark surfaces.

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (45 earlier review cycles; latest 8 shown)

• reviewed 2026-08-29T01:46:37.650Z sha 6baa16f :: needs changes before merge. :: [P3] Document the per-ID and aggregate limits
• reviewed 2026-08-29T04:56:44.530Z sha 6baa16f :: needs changes before merge. :: [P3] Document the per-ID and retained-ID limits
• reviewed 2026-08-29T06:04:05.314Z sha 6baa16f :: needs changes before merge. :: [P3] Document every enforced --ids limit
• reviewed 2026-08-29T09:08:56.877Z sha 6baa16f :: needs changes before merge. :: [P3] Document the per-ID and retained-ID limits
• reviewed 2026-08-29T12:00:22.634Z sha 6baa16f :: needs changes before merge. :: [P3] Document the per-ID and aggregate selection limits
• reviewed 2026-08-29T13:59:48.300Z sha 6baa16f :: needs changes before merge. :: [P3] Document every enforced --ids limit
• reviewed 2026-08-30T22:36:10.622Z sha 6baa16f :: needs changes before merge. :: [P3] Document every enforced --ids limit
• reviewed 2026-08-31T05:33:23.076Z sha 6baa16f :: needs changes before merge. :: [P3] Document every enforced --ids limit

[SebTardif]

@clawsweeper

[P2] Bound total retained ID bytes
The 5,520-item guard runs after each arbitrary ID has been inserted into both ids and seen; because the scanner permits records up to 1 MiB and IDs only reject whitespace, a hostile HTTP source can still retain several GiB across 5,520 unique records. Add an aggregate byte or ID-length bound that still accepts the documented

Pushed 6baa16f. One extracted id is capped at 256 bytes. Retained id text is capped at 256 KiB. That is not a JSONL file-size limit, so the documented 5,520-id / 1.3 MiB SkillTrustBench stream still parses.

@clawsweeper re-review

[SebTardif]

@clawsweeper

The PR has useful terminal proof at b501d31, but the final retained-text limits were added in 6baa16f and need a redacted current-head runtime transcript.

Refreshed the PR body with public CLI output from 6baa16f: 256-byte id reject, 262144-byte retained-id reject, 5520-id reject, and a 299600-byte JSONL source that loads then fails on a missing SkillTrustBench row. Also ran make docs-site (Built 7 docs page(s) in dist/docs-site); generated dist/ was not committed.

@clawsweeper re-review