BPB: use renderer-reported content bytes as the denominator#825
Draft
YujiaBao wants to merge 3 commits into
Draft
BPB: use renderer-reported content bytes as the denominator#825YujiaBao wants to merge 3 commits into
YujiaBao wants to merge 3 commits into
Conversation
The BPB denominator counted UTF-8 bytes of the decoded trained tokens. Registered special tokens are excluded (thinking-machines-lab#824), but scaffolding that renderers inject as ordinary text still counted as target bytes -- e.g. the <think></think> block Kimi renderers put in every trained assistant message is not in all_special_ids (nor is Qwen3's <think>). Scaffolding is near-zero-loss once the template is learned, so its bytes pad the denominator and deflate BPB by an amount that varies with the chat template, biasing exactly the cross-model comparisons BPB exists for. Fix by counting content bytes at the message level, where they are exact: - Renderers report per-message content bytes (text + rendered thinking + tool-call names/args) via RenderedMessage.content_byte_count, using the new message_content_byte_count helper. Token-level masks cannot express this: renderers deliberately encode scaffolding+content as one string so tokenization matches the HF chat template. - New Renderer.build_supervised_example_with_metadata returns a SupervisedExample carrying trained_content_bytes (sum over loss-weighted messages). The legacy build_supervised_example keeps its tuple signature; renderers that override only the legacy method still work and simply report no content bytes. - The count rides on the datum as DatumWithContentBytes, a tinker.Datum subclass with a plain attribute: the SDK serializes datums from model_input + loss_fn_inputs only, so the field never reaches the service (which restricts loss_fn_inputs keys). - compute_bpb uses content bytes when present: numerator = full trained NLL (scaffolding pays its real bits, which vanish as the template is learned), denominator = content bytes, identical across renderers for the same data. Datums without a count (custom renderers, or trained tokens lost to max_length truncation) fall back to the previous token-byte computation. pytest renderers/ supervised/ recipes/chat_sl/: 553 passed. ruff and pyright clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LC4d9VwvhNY3DX7EbmpMmv
Review fix: with build_supervised_example_with_metadata as a public override point, a subclass of an overriding renderer (e.g. a user's KimiK2Renderer subclass) that customizes the legacy build_supervised_example was silently bypassed by conversation_to_datum, which dispatched to the parent's metadata override instead -- different training tokens than before the metric change. There is now a single overridable implementation chain, _build_supervised_example_impl (matching the renderers' existing protected-hook style, e.g. _render_tool_calls); kimi_k2 / gpt_oss / nemotron3 override it instead of the public metadata method. The metadata method is a @Final dispatcher that compares MRO positions: if a legacy build_supervised_example override is more derived than any impl override, it is honored (its tokens/weights, content bytes None), so both public entry points always agree on the tokens. The legacy wrapper calls the impl chain directly, keeping super().build_supervised_example() from legacy overrides recursion-free and byte-identical to the pre-change behavior. Adds regression tests for a legacy override layered on KimiK2Renderer (now honored on both paths) and for impl-hook overrides reporting content bytes on both paths. tests/downstream_compat: 499 passed; renderers/supervised/chat_sl: 555 passed; ruff + pyright clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LC4d9VwvhNY3DX7EbmpMmv
…ron effort suffix, doc precision
Findings from an independent multi-lens review with adversarial
verification (13 confirmed findings deduplicating to three issues):
1. Dispatcher (major): the MRO-index rule honored a legacy
build_supervised_example override only when it was MORE derived than
any _build_supervised_example_impl override. In the reverse layering
(parent with a legacy override, child overriding the impl hook per the
new guidance), the metadata entry point skipped the parent's message
transformation while direct build_supervised_example calls applied it
-- silently diverging tokens. The dispatcher now honors ANY legacy
override below Renderer, which matches Python attribute lookup exactly
and makes the token-agreement guarantee unconditional: a legacy
override that calls super().build_supervised_example() still reaches
the most-derived impl, so impl overrides participate on both paths.
_mro_definer_index is gone. Regression test added for the layered
shape.
2. Nemotron-3 low/medium-effort renderers (minor): the injected
'{reasoning effort: ...}' user-message suffix was counted as content
bytes. It is now discounted after rendering (it still tokenizes inside
the message content exactly as the HF template requires). Test added.
3. Docs (minor): "denominator identical across renderers" is now stated
precisely -- the count is independent of template formatting, but
renderers that legitimately train on different content (e.g. gpt-oss
preserves historical thinking that Qwen3/Kimi strip) count different
bytes; and "scaffolding is never counted" is scoped to content-byte
mode, with the token-byte fallback (custom renderers, max_length
truncation) called out in the sweep README.
renderers/supervised/chat_sl: 557 passed; tests/downstream_compat: 499
passed; ruff + pyright clean.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LC4d9VwvhNY3DX7EbmpMmv
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.
Follow-up to #823 / #824.
Problem
The BPB denominator counts the UTF-8 bytes of the decoded trained tokens. #824 excluded registered special tokens from both sides, but scaffolding that renderers inject as ordinary text still counts as target bytes. The clearest case: Kimi renderers put a
<think></think>block in every trained assistant message, and<think>/</think>are not inall_special_idsfor the Kimi K2 tokenizer (where<think>is even three regular tokens:<,think,>) nor for Qwen3.Scaffolding is near-zero-loss once the template is learned, so its bytes act as free denominator padding: measured BPB ≈ true BPB × B/(B+C) for B content bytes and C scaffolding bytes per turn. On a short turn (
"2+2 equals 4."= 13 bytes), Kimi's trained span decodes to<think></think>2+2 equals 4.— 15 of 28 denominator bytes (54%) are scaffolding. The deflation varies with the chat template, biasing exactly the cross-model comparisons BPB exists for; templates with more markup look artificially better.Token-level filtering can't fix this: renderers deliberately encode scaffolding + content as one concatenated string so tokenization matches the HF chat template, so no exact per-token content mask exists (and BPE can merge across the boundary). The message level is where content bytes are exact.
Fix
Renderers report content bytes. New helper
message_content_byte_count(message, include_thinking=..., include_tool_calls=...)counts the payload that survives round-tripping throughparse_response: text parts, thinking parts (when actually rendered), and tool-call function names/arguments. Each built-in renderer setsRenderedMessage.content_byte_countusing flags that match what it actually rendered (e.g. thinking stripped from history → not counted; Nemotron's injected{reasoning effort: ...}suffix → discounted). For the same rendered content the count is independent of how the template formats it — which is what makes the denominator comparable. (Renderers that legitimately train on different content count different bytes: gpt-oss preserves historical thinking that Qwen3/Kimi strip, so cross-model comparisons are cleanest on data without thinking in historical turns; documented in the sweep README.)A new entry point carries the total; dispatch matches Python attribute lookup.
Renderer.build_supervised_example_with_metadatareturns aSupervisedExample(model_input, weights, trained_content_bytes)where the count sums over loss-weighted messages. It is a@finaldispatcher over a single overridable implementation chain,_build_supervised_example_impl(same protected-hook style as_render_tool_calls);kimi_k2/gpt_oss/nemotron3override the hook. The legacybuild_supervised_examplekeeps its tuple signature and routes into the same chain. If any class in the MRO overrides the legacy method — the pre-existing extension point, at any layer, including overrides that callsuper().build_supervised_example()— the dispatcher routes through it exactly as a direct call would (reportingNonefor content bytes), so the two public entry points return identical tokens in every hierarchy, and no existing subclass silently changes its training tokens.The count rides on the datum without touching the wire.
DatumWithContentBytesis atinker.Datumsubclass with a plain attribute — deliberately not aloss_fn_inputsentry: the SDK serializes datums frommodel_input+loss_fn_inputsonly (andforward_backward_customrejects unknownloss_fn_inputskeys), so the field never reaches the service and datums pass throughforward_backwardunchanged.datum_from_model_input_weightsdrops the count ifmax_lengthtruncation removes any loss-weighted token, since it would no longer describe the surviving span.compute_bpbprefers content bytes. For datums carrying a count:The numerator is the model's entire trained code length — scaffolding pays its real bits, which vanish as the template is learned and are charged symmetrically to every model (no dependence on which tokens happen to be registered as special). The denominator is fixed by the trained content, not the template. Datums without a count (custom renderers overriding only the legacy method, or trained spans cut by
max_length) fall back per-datum to the previous token-byte computation, so everything keeps working andtrain_mean_bpb/test/bpbkeep their names; the sweep README documents the fallback.With this, the same conversation scored under Kimi K2 and Qwen3 renderers yields the identical denominator (asserted in tests), and think-tag scaffolding remains trained but contributes no bytes.
Review
Beyond unit tests, the change went through an independent multi-lens review (metric math, dispatch/backward-compat, per-renderer accounting, SDK integration, test quality, docs accuracy, whole-diff pass) with two adversarial verifiers per finding. Confirmed findings — a dispatch edge where an impl-hook override layered above a legacy override diverged from direct-call behavior, the Nemotron effort-suffix bytes, and two over-broad doc claims — are all fixed in the last commit; the dispatcher now defers to any legacy override exactly as Python attribute lookup does.
Test
renderers/content_bytes_test.py(new): kimi/qwen3 agree on content bytes for the same conversation; Kimi's<think></think>is trained but not counted; thinking counted only where rendered (last message vs. stripped history, both renderers); untrained messages contribute nothing; role_colon separators excluded; both entry points return identical tokens/weights. Dispatch regression tests: legacy-only overrides fall back toNonewith identical tokens; a legacy override layered onKimiK2Rendereris honored on both entry points; an impl-hook override layered above a legacy override still agrees;super().build_supervised_example()from a legacy override is recursion-free; impl-hook overrides report content bytes on both paths.renderers/nemotron3_test.py: the rendered{reasoning effort: low}suffix is excluded from content bytes.supervised/common_test.py: content-byte mode hand computation (numerator includes trained special tokens); weight-magnitude invariance (mean reduction); scaffolding pads the fallback denominator but not the content one; mixed batches fall back per-datum; length-mismatch raises;DatumWithContentByteskeepsloss_fn_inputsto exactly{weights, target_tokens}, pickles, and survives mean reduction; truncation into the trained region drops the count while truncation of untrained tokens keeps it.model_input+loss_fn_inputsonly.uv run --extra dev pytest tinker_cookbook/renderers/ tinker_cookbook/supervised/ tinker_cookbook/recipes/chat_sl/→ 557 passed, 46 skipped (skips pre-existing, HF-gated);tests/downstream_compat/→ 499 passed;ruff check/ruff format --checkclean;pyright0 errors on all touched files.🤖 Generated with Claude Code
https://claude.ai/code/session_01LC4d9VwvhNY3DX7EbmpMmv