Skip to content

Include captured conditioning in sampled-source identity - #886

Draft
bradhilton wants to merge 1 commit into
schulman/883-causal-history-coverage-20260910from
schulman/883-captured-source-identity-20260910
Draft

Include captured conditioning in sampled-source identity#886
bradhilton wants to merge 1 commit into
schulman/883-causal-history-coverage-20260910from
schulman/883-captured-source-identity-20260910

Conversation

@bradhilton

Copy link
Copy Markdown
Collaborator

When captures reuse response metadata and generated tokens but have different captured prompts, source deduplication can discard a valid sampled prefix. Include captured prompt IDs in the existing fingerprint for Chat, Completions, Responses and Messages, following each protocol's extraction precedence. Identical copies still deduplicate; unrelated choices do not change the selected source identity.

For example, captures [1] → [2] and [1, 2, 3] → [2] with identical response IDs/logprobs now retain both sampled prefixes. This changes one runtime function; public tokenization arguments, loss normalization and reconciliation policy are unchanged.

Stacked on #885; partial follow-up for #883. The documented explicit/manual changed-conditioning and renderer-repair counterexamples remain unresolved, so this PR does not close #883. Held for morning merge review; existing research drivers require separate source resealing before adoption.

Validation: independent review at exact head 9fe17e39db8ad237ae304920b158b69f2f66ebf9 passed all 484 trajectory tests and 21 additional duplication/retention controls. The 16 new tests produce 10 expected failures and six passing controls on the parent. Two real captured trajectories retain exact tensors and 7,445/11,685 selected tokens; seven policy controls preserve the known scope limits. Ruff and format checks pass. Independent review.

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

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