Skip to content

Preserve distinct captures with different prompt tokens - #886

Merged
bradhilton merged 1 commit into
mainfrom
schulman/883-captured-source-identity-20260910
Sep 10, 2026
Merged

bradhilton merged 1 commit into
mainfrom
schulman/883-captured-source-identity-20260910

Conversation

@bradhilton

@bradhilton bradhilton commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator

Captures with different original prompts can be mistaken for duplicates when their response identifiers, generated tokens, and output metadata match. Include captured prompt token IDs in the existing source fingerprint so both training examples survive. Genuine copies of the same capture still deduplicate.

This version is standalone on ART main 9a16366d79eebc36e00f75670965e2629c012b75. It contains only the source-identity change from the previous #886: one runtime function in art.trajectories._tokenize, its explanatory comment, and tests. _history.py is byte-identical to main; #885's automatic history-attribution change is excluded. Public APIs, automatic history construction, explicit reconciliation, manual-history editing, and loss normalization retain main's behavior. No new edited-history validator or probability recomputation is introduced. The broader history-attribution discussion in #883 remains separate.

Validation on the standalone source:

  • 547 trajectory tests pass, including all 16 source-identity tests. On unchanged main, those 16 produce 10 expected failures and six passing controls.
  • Exact diff/AST checks prove the only runtime function change is the same fingerprint function as the original Preserve distinct captures with different prompt tokens #886, with captured-prompt extraction precedence preserved for Chat, Completions, Responses and Messages.
  • Ruff, formatting and lock checks pass. Local full type checks report the same 11 diagnostics on unchanged main and this candidate; hosted CI is tracked separately.
  • Independent standalone review passes nine extra identity controls and 11 manual/edit/reconciliation cases whose native outputs are byte-identical to main. Fresh formal reviews from Minsky and McCarthy are requested for head 3627311444748067482c486909d3642644509efd; earlier stacked-head approvals remain historical evidence.

Prepared at Brad's request. This does not merge #885, relax experiment source fences, launch research, or authorize deployment.

@bradhilton bradhilton left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review text removed on 2026-09-17; the consolidated review record is in the summary comment on this PR.

@bradhilton
bradhilton force-pushed the schulman/883-captured-source-identity-20260910 branch from 9fe17e3 to 3627311 Compare September 10, 2026 18:50
@bradhilton
bradhilton had a problem deploying to trainer-rank-gpu-validation September 10, 2026 18:51 — with GitHub Actions Error
@bradhilton bradhilton changed the title Include captured conditioning in sampled-source identity Preserve distinct captures with different prompt tokens Sep 10, 2026
@bradhilton
bradhilton changed the base branch from schulman/883-causal-history-coverage-20260910 to main September 10, 2026 18:51
@bradhilton bradhilton closed this Sep 10, 2026
@bradhilton bradhilton reopened this Sep 10, 2026

@bradhilton bradhilton left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review text removed on 2026-09-17; the consolidated review record is in the summary comment on this PR.

@bradhilton bradhilton left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review text removed on 2026-09-17; the consolidated review record is in the summary comment on this PR.

@bradhilton
bradhilton marked this pull request as ready for review September 10, 2026 19:33
@bradhilton
bradhilton merged commit 66f644d into main Sep 10, 2026
17 of 20 checks passed
@bradhilton
bradhilton deleted the schulman/883-captured-source-identity-20260910 branch September 10, 2026 19:36
@bradhilton

Copy link
Copy Markdown
Collaborator Author

Consolidated review record (automated agent review traffic removed 2026-09-17)

Change. Captures with different original prompts but matching response IDs, generated tokens and output metadata were mistaken for duplicates, dropping a training example. The sampled-source fingerprint in art.trajectories._tokenize now includes captured prompt token IDs for all four protocols, so distinct prompts survive while genuine copies still deduplicate; standalone on main, excluding #885.

Review. Minsky and McCarthy each posted fresh CLEAR verdicts at the merged head 3627311, after earlier clearing the stacked head. Verified: 547 trajectory tests pass plus 21 extra identity controls; the 16 new identity tests give 10 expected failures on unchanged main; ruff/format clean; the two-file patch matches the original delta with _history.py unchanged. Schulman confirmed correct-base CI after reopening: the GPU classifier gate passed (GPU job skipped), Prek still in progress.

Deferred / follow-ups.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant