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

Preserve distinct captures with different prompt tokens#886
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.

Minsky review — CLEAR at 9fe17e39db8ad237ae304920b158b69f2f66ebf9 (parent = #885 head da4f165a). Scope is sampled-source identity only; #883's reconcile/manual/repair gaps remain open and no research source adopts this.

Finding: _sampled_evidence_fingerprint previously hashed only output-side evidence (message/text/blocks, token_ids, logprobs, finish/stop reason), so two exchanges with a reused response ID and identical outputs but different captured prompts collapsed into one source key and one causal prefix was dropped. The head adds prompt_token_ids to the fingerprint for all four protocols: Chat and Completions take the choice-level value and fall back to the response-level value when the choice's is missing or null (the same precedence _chat_choice_tokens uses), Responses takes the generation-level prompt_token_ids when present, Messages takes the response extra. Identical captures still hash identically, so legitimate deduplication is unchanged; only distinct causal contexts now stay distinct. The comment on _source_key states the rationale.

Verification at this exact head (ART venv):

  • tests/unit/trajectories: 484 passed. Ruff check and format clean on both changed files.
  • Negative control: the new test_sampled_source_identity.py against da4f165a → 10 failed / 6 passed. Failing: distinct-prompt binding (all protocols where the response ID is reused), the missing/null choice-prompt fallback cases, and test_reused_response_identity_preserves_both_sampled_causal_prefixes[False/True]. The six that pass on the parent are the identical-capture roundtrip and unrelated-choice controls, which by design do not depend on the change.
  • The runtime delta is 15 lines confined to the fingerprint; no tokenizer, renderer, history-splitting or API behaviour changes.

Draft, stacked on #885 and held; McCarthy has cleared this same SHA, so both dedicated-reviewer pass-offs are on 9fe17e39. CI still pending per the thread.

@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.

Minsky fresh review — CLEAR at 3627311444748067482c486909d3642644509efd (standalone on main 9a16366d). This is a new exact-head verdict; my earlier clearance of the stacked head 9fe17e39 does not transfer by itself.

Base facts verified: parent is 9a16366d (main, which now includes #882 and #887); git patch-id of 9a16366d..36273114 equals that of the cleared stacked delta da4f165a..9fe17e39 (9e17ab53…), so the change is byte-for-byte the same 15-line fingerprint edit plus the same 160-line test module; _history.py is byte-identical to main; #885's head da4f165a is not an ancestor, so no #885 default-path splitting, manual-edit validator or attribution behaviour is imported.

Change (unchanged from prior review): _sampled_evidence_fingerprint includes captured prompt_token_ids for all four protocols (choice-level with response-level fallback for Chat/Completions, generation-level for Responses, response extra for Messages), so reused response IDs with identical outputs under different captured prompts stay distinct sources while identical captures still deduplicate. No tokenizer, renderer, history-splitting or API change.

Verification at this exact head (ART venv):

  • tests/unit/trajectories: 547 passed. Ruff check and format clean on both files.
  • Negative control: the head's test_sampled_source_identity.py against main 9a16366d → 10 failed / 6 passed (same shape as on the stacked base: distinct-prompt binding, choice-prompt fallback and reused-identity cases fail; roundtrip and unrelated-choice controls pass), matching the PR's stated 10/6 → 16/16.

Draft; McCarthy is reviewing this same SHA. Merge/adoption remain Brad's decision; #885 stays held separately.

@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.

McCarthy fresh standalone review: CLEAR, no blocking findings at 3627311444748067482c486909d3642644509efd against main 9a16366d79eebc36e00f75670965e2629c012b75.

Captured prompt IDs now distinguish sampled sources whose response IDs, output tokens and other evidence collide. Chat/Completions retain choice-first, response-fallback precedence (including missing/null fallback); Messages use response metadata and Responses use the selected generation. Genuine copies still deduplicate, and unrelated choices/generations do not affect the selected source. The change reaches existing source identity comparisons without introducing an edited-history validator or changing probability/normalization policy.

Standalone composition verified: exactly two changed files; _history.py is byte-identical to main and #885 is not an ancestor. The fingerprint function and added test file match the old reviewed #886 bytes. Replacing that one function with its main version reproduces the complete main module AST; the standalone patch ID matches the original identity-only delta.

Independent private-copy validation:

  • 547 trajectory tests plus 21 additional identity/deduplication controls passed (568 total). On main, the new 16 identity cases give exactly 10 expected failures / 6 passes; the 21 additional controls also pass there. Ruff/format clean.
  • 36 default/reconciliation/roundtrip/manual cases have byte-identical actual outcomes between main and this head (26 successful outcomes, 10 matching refusals). Three additional successful native manual-edit controls also match exact token/flag/logprob values, covering user-authored unsourced messages, removing earlier turns, and edited Completions prompts. Network access was forbidden for these comparisons.
  • All 1,189 archived source files rehash unchanged. Tokenizer SHA256: 1917cb970fc88f9a9227bc6dc119b8429e1b8350e31e5eacd203dcfad2d76c88.

These are bounded behavior comparisons, not proof of arbitrary manual-history causal correctness; the broader #883 limitations remain separate. Captured-prompt hashing adds work proportional to prompt length; no performance qualification is claimed here. Fresh hosted CI for the retargeted main base remains separate/pending; old-base event statuses are not its receipt. No research adoption, source-fence override, merge or deployment performed.

Evidence: /home/brad/.local/share/mccarthy/art886-standalone-review-20260910/REVIEW.md, source proof, test logs and behavior receipts.

@bradhilton

Copy link
Copy Markdown
Collaborator Author

Schulman: Standalone revision is now 3627311444748067482c486909d3642644509efd, directly on main 9a16366d79eebc36e00f75670965e2629c012b75. Both Minsky and McCarthy have posted fresh CLEAR reviews for this exact head. _history.py matches main, #885 is not an ancestor, and the two-file patch contains only the original source-identity fix and tests. Eleven independent manual/edit/reconcile cases have byte-identical native outputs, supplemented by McCarthy's 36-case main/candidate comparison and three native manual-edit cases.

The first push-triggered CI event captured the previous stacked base before retargeting. The same draft PR was closed/reopened with the source commit unchanged to obtain correct-base CI. Do not treat the obsolete event runs as standalone clearance. Replacement GPU classifier run34517235201 explicitly uses base9a163/head362731, reports validation not required, and passes its gate with the GPU job skipped. Correct-base Prek34517235910 remains in progress.

Obsolete runs34516840799/34516840907 are canceled. The old GPU job has a recorded termination receipt and its exact pod trainer-rank-gpu-34516840907-1-37eb12da-head / UID852bba02-e96c-4878-972b-6a206f07cc31 is now absent. This was cleanup, not a GPU test failure or successful qualification. No research, merge or deployment occurred.

@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
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