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 real behavior proof before merge. Reviewed August 28, 2026, 12:36 AM ET / 04:36 UTC.

ClawSweeper review

What this changes

This PR streams SkillTrustBench --ids input from local or HTTP sources, caps retained identifiers, and adds coverage and documentation for the new loading behavior.

Merge readiness

⚠️ Ready for maintainer review - 4 items remain

Keep open: current main still fully buffers --ids sources, while this PR narrowly streams them and bounds retained IDs. The implementation is sound, but the new per-ID and retained-ID limits need complete operator documentation before merge.

Priority: P2
Reviewed head: 6baa16f5fe70e10ca1f69925ae1fe25d034a2b9a

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	PR readiness rating was derived from proof quality, review findings, security review, and reviewer confidence.
Proof confidence	🦞 diamond lobster (5/6)	Sufficient (terminal): Exact-head public-CLI terminal evidence shows each new rejection limit and shows a JSONL source larger than 256 KiB advances past selection loading.
Patch quality	🐚 platinum hermit (4/6)	1 actionable review finding remain.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (terminal): Exact-head public-CLI terminal evidence shows each new rejection limit and shows a JSONL source larger than 256 KiB advances past selection loading.
Evidence reviewed	7 items	Current-main behavior: The fetched current-main revision still uses io.ReadAll for HTTP and os.ReadFile for local --ids sources; the head replaces both with an io.ReadCloser consumed by the existing scanner. Introduced streaming and bounded retention: The introduced loader opens either source as a reader, closes it after parsing, rejects IDs over 256 bytes, caps retained ID text at 256 KiB, and caps selections at 5,520 IDs. Focused coverage: New tests cover a JSONL source above 256 KiB over both file and HTTP paths plus individual-ID, aggregate-retained-text, and count-limit failures.
Findings	1 actionable finding	[P3] Document every enforced --ids limit
Security	None	None.

Live Verification

Command: go run ./cmd/clawscan --help

Result: FAIL (failed) — execution before step 1 expect_output: sh -lc pnpm install --ignore-scripts --frozen-lockfile failed: ! Corepack is about to download https://registry.npmjs.org/pnpm/-/pnpm-11.24.0.tgz | [ERR_PNPM_NO_PKG_MANIFEST] No package.json found in /tmp/clawsweeper-live-proof-47-pbvJAk/target

sh -lc pnpm install --ignore-scripts --frozen-lockfile failed: ! Corepack is about to download https://registry.npmjs.org/pnpm/-/pnpm-11.24.0.tgz | [ERR_PNPM_NO_PKG_MANIFEST] No package.json found in /tmp/clawsweeper-live-proof-47-pbvJAk/target

Assertions:

• FAIL expect_output: Benchmark command flags:
• FAIL expect_output: --ids ‹path-or-url›

How this fits together

The benchmark command reads an optional ID list before the SkillTrustBench adapter fetches benchmark rows and scans selected cases. This loader controls memory use and determines which IDs reach the benchmark runner.

flowchart LR
  A[Benchmark command] --> B[File or HTTP ID source]
  B --> C[Streaming ID loader]
  C --> D[ID validation and limits]
  D --> E[Pinned SkillTrustBench rows]
  E --> F[Selected case scans]
  F --> G[Benchmark artifact]

Loading

Before merge

• Add real behavior proof - Sufficient (terminal): Exact-head public-CLI terminal evidence shows each new rejection limit and shows a JSONL source larger than 256 KiB advances past selection loading.
• Document every enforced `--ids` limit (P3) - The loader now rejects IDs over 256 bytes and selections retaining over 262,144 bytes, but this changed documentation only states the 5,520-ID limit. Document both byte limits in the public benchmark docs and CLI skill so operators do not discover a new input contract only at runtime.
• Resolve merge risk (P1) - Merging introduces 256-byte per-ID and 256 KiB aggregate retained-ID constraints that prior --ids sources did not advertise, so affected operators could encounter a new validation error without complete documentation.
• Complete next step (P2) - A small, mechanical documentation repair resolves the sole remaining actionable finding without changing the loader design.

Findings

• [P3] Document every enforced --ids limit — docs/benchmarks.md:14-17

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Change mix	runner/CLI +34/-10, tests +85, docs/skill +5/-2	Most growth is focused regression coverage for a small loader change, with documentation updates in the same PR.

Root-cause cluster

Relationship: canonical
Canonical: #47
Summary: This PR is the viable follow-up to the closed unmerged HTTP-only cap proposal and addresses the same loader memory problem without rejecting valid large JSONL streams.

Members:

• superseded: fix(runner): cap HTTP --ids response bodies #45 - That closed unmerged proposal applied an HTTP body-size cap; this PR replaces it with streaming and semantic retention limits for both source types.

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

Merge-risk options

Maintainer options:

1. Document the new input contract (recommended)
   State the 256-byte per-ID and 256 KiB retained-ID limits beside the existing 5,520-ID limit, while clarifying that these do not cap JSONL source size.

Copy recommended automerge instruction

@clawsweeper automerge

Special instructions:
Document the 256-byte per-ID and 256 KiB retained-ID limits in docs/benchmarks.md and skills/clawscan-cli/SKILL.md, and retain the clarification that the JSONL stream itself has no size cap.

Technical review

Best possible solution:

Keep the streaming implementation and document all three semantic limits consistently in the benchmark docs and CLI skill, while explicitly preserving that JSONL source size itself is not capped.

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

Yes, at source level: fetched current main fully buffers both local and HTTP --ids sources, and the PR head replaces those paths with a streaming reader. The read-only review did not execute a new local reproduction.

Is this the best way to solve the issue?

No, not quite: streaming through the existing parser with retained-ID bounds is the narrow maintainable repair, but the public contract must also disclose both newly enforced byte limits.

Full review comments:

• [P3] Document every enforced --ids limit — docs/benchmarks.md:14-17
  The loader now rejects IDs over 256 bytes and selections retaining over 262,144 bytes, but this changed documentation only states the 5,520-ID limit. Document both byte limits in the public benchmark docs and CLI skill so operators do not discover a new input contract only at runtime.
  Confidence: 0.99

Overall correctness: patch is correct
Overall confidence: 0.96

AGENTS.md: found and applied where relevant.

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

Labels

Label justifications:

• P2: This is a bounded benchmark-loader memory and compatibility repair with limited blast radius.
• merge-risk: 🚨 compatibility: The new per-ID and aggregate-ID validation rules can reject previously accepted source files and URLs.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
• status: needs maintainer proof decision: A ClawSweeper-authored PR needs a maintainer proof capture or override decision. Sufficient (terminal): Exact-head public-CLI terminal evidence shows each new rejection limit and shows a JSONL source larger than 256 KiB advances past selection loading.
• proof: sufficient: Contributor real behavior proof is sufficient. Exact-head public-CLI terminal evidence shows each new rejection limit and shows a JSONL source larger than 256 KiB advances past selection loading.

Evidence

Acceptance criteria:

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

What I checked:

• Current-main behavior: The fetched current-main revision still uses io.ReadAll for HTTP and os.ReadFile for local --ids sources; the head replaces both with an io.ReadCloser consumed by the existing scanner. (internal/runner/benchmark.go:322, f3ac276393cd)
• Introduced streaming and bounded retention: The introduced loader opens either source as a reader, closes it after parsing, rejects IDs over 256 bytes, caps retained ID text at 256 KiB, and caps selections at 5,520 IDs. (internal/runner/benchmark.go:345, 6baa16f5fe70)
• Focused coverage: New tests cover a JSONL source above 256 KiB over both file and HTTP paths plus individual-ID, aggregate-retained-text, and count-limit failures. (internal/runner/runner_test.go:804, 6baa16f5fe70)
• Undocumented compatibility limits: The changed public benchmark documentation and CLI skill mention streaming and the 5,520-ID maximum, but omit the enforced 256-byte per-ID and 262,144-byte retained-ID limits. (docs/benchmarks.md:14, 6baa16f5fe70)
• Main and release status: The PR head is not an ancestor of fetched main, whose parent is the PR base; the provided latest release is v0.1.6, predating this August PR, so neither current main nor the release already supplies this fix. (internal/runner/benchmark.go:327, f3ac276393cd)
• Feature history: Benchmark history identifies Patrick Erichsen as the primary long-running contributor to this file, while SebTardif authored the immediately preceding current-main runner fix and this focused follow-up. (internal/runner/benchmark.go:322, 113c1a8ede9f)

Likely related people:

• Patrick Erichsen: History shows the largest number of prior changes to the benchmark loader, including the pinned SkillTrustBench source work. (role: benchmark feature owner; confidence: high; commits: e6c131716384, bf6f4f125782; files: internal/runner/benchmark.go)
• SebTardif: Authored the immediately preceding runner change on the PR base and the current streaming/bounds implementation. (role: recent runner contributor; confidence: high; commits: 113c1a8ede9f, 6baa16f5fe70; files: internal/runner/benchmark.go, internal/runner/runner_test.go)

Rank-up moves

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

• Tighten the PR description with what changed, how it was validated, and any remaining risk.

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 (35 earlier review cycles; latest 8 shown)

• reviewed 2026-08-25T18:52:39.972Z sha 6baa16f :: needs maintainer review before merge. :: none
• reviewed 2026-08-25T21:46:57.577Z sha 6baa16f :: needs maintainer review before merge. :: none
• reviewed 2026-08-26T03:13:29.992Z sha 6baa16f :: needs maintainer review before merge. :: none
• reviewed 2026-08-26T08:10:48.123Z sha 6baa16f :: needs maintainer review before merge. :: none
• reviewed 2026-08-26T10:04:44.066Z sha 6baa16f :: needs maintainer review before merge. :: none
• reviewed 2026-08-26T18:05:54.333Z sha 6baa16f :: needs maintainer review before merge. :: none
• reviewed 2026-08-27T03:31:09.052Z sha 6baa16f :: needs changes before merge. :: [P3] Document every enforced --ids limit
• reviewed 2026-08-28T02:48:25.295Z sha 6baa16f :: needs real behavior proof before merge. :: [P3] Document the per-ID and retained-ID limits

[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