Repository navigation
PR #801 segmentation results DO NOT MERGE - #824
Closed
ttlequals0 wants to merge 23 commits into
Closed
ttlequals0 wants to merge 23 commits into
ttlequals0 wants to merge 23 commits into
Conversation
ttlequals0
changed the title
ttlequals0
changed the title
cli.run/report/show-prompt/dump-prompt gain --prompt-variant (default detection), validated the same way as --addressing-mode. Segmentation uses its frozen prompt and rejects --snapshot. runner._build_user_prompt, precompute_prompt_hashes, reconstruct_user_prompt, and run() thread prompt_variant through to variants.format_segmentation_prompt / parse_segmentation_response; the call record and derive_episode_results rows carry it alongside addressing_mode, with legacy defaults for old records. cli.run also gains repeatable --model/--episode filters, applied before the dry-run preview.
report.render() now filters by prompt_variant (default 'detection', legacy records untagged) alongside addressing_mode, with matching title suffix and metadata line. Non-default (variant, mode) cells write to suffixed report/assets paths via a new report_paths() helper, wired into cli.run/cli.report; charts in those reports link to their own assets dir. Adds `benchmark compare`, writing results/comparison.md with one row per model present in >=2 of the four cells, reusing aggregate.py's per-model scoring, plus a paired delta/p-value between segmentation/segment_ids and detection/timestamps (reusing the TL;DR tier t-test's diff/stat computation).
Stdlib script that appends another checkout's call records and response shards under a prompt variant, skipping call ids already present.
Add a README subsection for the detection/segmentation prompt variants and commit the regenerated segmentation+segment_ids report cell.
Critical/important fixes from the prompt-variant-benchmark review: - Unparseable segmentation responses now report method=None (counted as a parse failure) instead of the string "none" (scored 0.5); an all-main-content response now scores full compliance for its addressing mode instead of the raw extraction method. - The id-mode int conversion no longer raises on "1.0" or inf/nan values. - benchmark compare divides cost by the cell's own episode count per model, so cells with different episode counts are comparable; it also excludes deprecated models, matching the per-cell report. - Added known-value tests for the paired t-test p-value math. Minor cleanup: mixed int/float id-mode entries log a warning instead of dropping silently; ADDRESSING_MODES defined once in variants.py; compressed over-long docstrings; moved inline test imports to the top of the file; replaced a try/assert False test pattern with pytest.raises; import_calls.py JSON decode errors now name the file and line; added CLI tests for show-prompt/dump-prompt with the segmentation variant; corrected the README's imported-rows hash-verification caveat with exact counts; added a CHANGELOG entry for the prompt-variant benchmark feature.
results/raw/calls.jsonl was approaching GitHub's 100 MB blob limit. Call records now live in results/raw/calls/<model>.jsonl, mirroring the existing per-model response shards; schema_version bumps to 3 (readers still accept v2). Adds `benchmark migrate-calls` to fold an existing flat calls.jsonl into shards, idempotently.
Three rendered strings and several cli.py/runner.py help texts and docstrings still named the deleted flat calls.jsonl; reworded to the per-model shard layout, leaving the legacy-file mentions inside migrate-calls/migrate-raw and the read_calls fallback warning alone since those genuinely describe that file. Also fixes a latent tie-break: the detection-by-length/position charts sorted models by detection rate with ties broken by input order, which silently depended on calls.jsonl's row order; now explicitly tie-broken alphabetically so it's stable regardless of read order. CHANGELOG notes the schema v3 shard move. Regenerated report.md, the segmentation/segment_ids report, and comparison.md so committed output matches the new wording and chart ordering.
Move every existing row of calls.jsonl into results/raw/calls/<model>.jsonl so no single data file grows past GitHub's 100 MB blob limit.
Append the 1880 call records and response-shard lines PR #801 ran under the segmentation prompt variant. Co-authored-by: John Belcher <[email protected]>
Current-prompt rows for three new models and segmentation-prompt rows for fourteen models on the eight-episode subset, with the regenerated default and segmentation cell reports, the comparison, and a fresh pricing snapshot.
The Claude wrapper's five-hour session cap was returning HTTP 429 on every call until reset, each recorded as an error needing a --retry-errors pass. Detect that wrapper-specific 429 shape (error.code == assistant_rate_limit or a resets_at field), pause the whole provider until resets_at + 15s (or Retry-After, or 60s) via a shared per-provider resume timestamp, and resend without consuming retries or writing an error row. A wait beyond the new [run] max_rate_limit_pause_seconds cap (default 6h) still records an error. OpenRouter-style 429s (no code, no resets_at) keep the prior backoff/error path. The OpenAI SDK client is now built with max_retries=0 so our own retry logic sees the raw 429 instead of the SDK silently retrying into the pause window.
_wait_for_pause slept once for a fixed duration, so a second 429 that extended the pause while a caller slept was ignored and it resent too early. Loop instead: re-read resume_at after every wake and only return once it is no longer in the future. Building the OpenAI client with max_retries=0 also dropped its inner retries for connection errors, timeouts, and 5xx, not just the 429 retry it was meant to disable. Restore those inside _call_openai_compatible via a small helper that retries them up to twice with the SDK's own backoff shape (0.5s doubling, capped at 8s, jittered) through the injectable sleep; 429s still go straight to the account-limit/transient handling without inner retries.
claude-fable-5-1 reached 8 of 15 episodes before the provider's session rate limit stopped it, and qwen/qwen3-8b was deprecated upstream with 7 of 15 segmentation episodes done. Both incomplete cells are removed; qwen3-8b's older detection rows stay. The two rate-limited mistral-small segmentation windows were re-run successfully. All three reports are regenerated from the remaining rows.
The System One and jev changes landed on main against the flat results/raw/calls.jsonl store. After rebasing onto them, the combined report command and the jev cache path still passed calls_path, and the two native accounting tests wrote and read the flat file. Point all four at the per-model shards under results/raw/calls/.
ttlequals0
force-pushed
the
feature/prompt-variant-benchmark
branch
from
October 13, 2026 12:01
83b7e57 to
e85677d
Compare
…eparately The detection runs added for the prompt-variant comparison had regenerated results/report.md and its charts. That file is the last full campaign and the next one is weeks out, so it returns to the main version. The regenerated detection-cell report is kept as results/report-prompt-testing.md with its own chart directory, labeled as the PR #801 / #824 prompt test.
- Release a paused provider's concurrency slots: call_with_retry records the pause and raises ProviderPausedError; the runner waits outside the semaphores and resends. - Dispatch prompt formatting and response parsing per variant through one table instead of string checks at each call site. - Apply --model/--episode as a work-unit selection inside the runner so cfg and episodes keep one scope. - Move call-store layout knowledge (reset, has-calls, batched dedup append) into storage; migrate-calls and import_calls share the append and write each shard once. - Derive report title, run hint and no-data note from one non-default cell list; share the paired-diff helper between compare and aggregate. - Warn about a legacy calls.jsonl once per path, not once per read.
Every budget gets its percentage and the 'of calls' qualifier closes the list, instead of a first-element special case in the loop.
- import_calls writes response lines before call rows, so a missing source shard fails before anything is committed, and validates the --variant value against the known prompt variants. - --model only accepts active model ids; a deprecated id used to pass validation and silently produce zero work units.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
PR #801 proposes a much shorter ad-detection system prompt: classify every transcript segment and keep the promotional ones. Its numbers compared the new prompt in segment-id mode against the old sweep in timestamp mode, and the harness could not hold two prompts side by side. This PR adds a prompt-variant dimension to the benchmark, imports the #801 rows, and runs 17 models over the full 15-episode corpus under both prompts so everything is in one place.
Reports in this PR:
results/report.mdstays at the last full campaign. The detection runs from this comparison are saved as the prompt-testing report.What changed
--prompt-variant {detection,segmentation}onrun,report,show-promptanddump-prompt.detectionis the live prompt and stays byte-identical, including every existingprompt_hash.segmentationis the Experiment: Prompt optimization #801 prompt frozen atprompts/segmentation-v1.txt, with its own user-prompt builder and a parser that keeps sponsor, cross_promo, self_promo and interaction segments.benchmark comparecommand (paired F0.5 deltas and p-values),--model/--episode/--retry-errorsrun filters, andmax_tokensrecorded on every call.[run] max_rate_limit_pause_seconds. A paused call gives up its concurrency slot, so other providers keep running.scripts/import_calls.pyand the 1880 imported Experiment: Prompt optimization #801 rows in a co-authored commit. Theepisode_results.jsonlrewrite in that commit is our schema change, not the contributor's.results/raw/calls/(schema v3). The flatcalls.jsonlcrossed GitHub's 100 MB blob limit with these rows.benchmark migrate-callsconverts an existing checkout.Findings
Paired on the 13 ad-bearing episodes. Delta is segmentation minus current.
How to read p: the chance of a gap at least this large if the prompt made no difference and the spread were episode-to-episode noise. It comes from a paired t-test over the per-episode F0.5 differences, the same test the report uses for its tiers. Below 0.05 counts as real.
Caveats
Verification
uv run pytest -qinbenchmarks/llm: 392 passed. Ruff reports no new findings on the touched files. Regenerating the segmentation cell report and the comparison from the committed rows reproduces the committed files apart from the generation timestamp.