Skip to content

refactor(tito): allowed append roles become a pure harness gate - #1778

Closed
guapisolo wants to merge 9 commits into
feat/tito-default-append-tool-userfrom
feat/tito-append-roles-harness-only
Closed

refactor(tito): allowed append roles become a pure harness gate#1778
guapisolo wants to merge 9 commits into
feat/tito-default-append-tool-userfrom
feat/tito-append-roles-harness-only

Conversation

@guapisolo

@guapisolo guapisolo commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

Depends on: #1777
Stacked on the parent PR's branch. Review / merge the parent first.

Summary

Template resolution becomes role-independent; append roles only gate harness behavior.

Motivation

resolve_fixed_chat_template keyed template rows on allowed_append_roles, forcing every new append shape (mid-session system injection, assistant input for compaction or few-shot) through template-registration ceremony. With preserve-think kwargs pinned as family constants, one template per family serves every role set. Default-config renders stay unchanged: after #1777 they already rode the multi-role rows this PR folds down.

Before / After

  • Before / After: before, each family registered SUPPORTED_TEMPLATES rows per role set, resolved by smallest superset; after, each family registers one FixedTemplate, resolved by resolve_fixed_chat_template(tito_model) with no roles argument.
  • What moved where: preserve-think kwargs fold from the multi-role rows onto the single FIXED_TEMPLATE; tokenize_additional_non_assistant becomes tokenize_additional_messages; VALID_APPEND_ROLES (now including assistant) becomes the single source for the CLI choices.
  • What moved where: qwen3.5_fixed.jinja renders mid-session system messages instead of raising; the vendored dsv32 reasoning-required raise narrows to the upstream drop_thinking=True path; MiniMax tool-only configs swap from the HF-native template to the fixed jinja.
  • What moved where: injected assistant input joins the prompt region of the next sample; the loss mask only covers generated response tokens, so it trains with loss 0 without new masking code.

Behavior Preservation

  • How we know: per-family byte tests prove intermediate-system, injected-assistant, assistant-then-user appends extend renders byte-for-byte on all nine jinja/HF families plus both DeepSeek encoder bridges.
  • How we know: MiniMax native-vs-fixed renders verified byte-identical on tool-loop histories regardless of the generation prompt; Kimi K2.5/K2.6 are untouched because their single {tool, user} row already served every subset request.
  • How we know: explicit tool-only configs change render bytes only when the initial prompt carries historical user/assistant thinking turns — the folded kwargs previously applied to multi-role surfaces only.

Verification

  • Existing test suite: 3015 fast tests pass across tests/fast/utils, tests/fast/router, tests/fast/rollout; test_fixed_templates.py is rewritten as a full family matrix over the role-free resolver.

Review Focus

  • Scrutinize the collapsed per-family FIXED_TEMPLATE kwargs against the previous multi-role rows.
  • Scrutinize the qwen3.5_fixed.jinja mid-session system block.
  • Scrutinize the narrowed dsv32 raise gate.

guapisolo and others added 5 commits July 23, 2026 22:27
Add a manual TP8/EP8 session test and expose the fixed EAGLE serving recipe through ModelConfig.

Keep SGLang TP/DP/PP/EP namespace fields on Miles' short-name contract, and forward the effective V3.2 thinking mode so SGLang separates reasoning before TITO comparison.
Remove the old-to-new mapping table and select tp_size, dp_size, pp_size, or ep_size directly from SGLang's registered aliases without changing the CLI surface.
Document that argparse derives its destination from the first alias, which is why Miles selects SGLang's short parallel-size field names.
…lates/

Copy sglang's encoding_dsv32/encoding_dsv4 (0.5.14.dev37+gf8cfad3) into
miles/utils/chat_template_utils/templates/ verbatim (provenance header only)
and route the deepseek bridge plus the test reference encoders through the
vendored modules. Miles-side render modifications land separately; the
sglang protocol.Tool seam stays on sglang so tool schemas keep serializing
with the server's defaults. The vendored files are excluded from pre-commit
formatters so they stay byte-diffable against upstream.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tool,user} surface

Upstream encoding_dsv32 gates every thinking block on last_user_idx, so a
new user turn strips historical think from the render and rewrites the
prefix — the reason V3.2 registered only the {tool} surface. Mirror
encoding_dsv4's gates in the vendored copy: drop_thinking=False now keeps
every assistant's thinking block and renders <think> openers position-
independently, while drop_thinking=True paths stay byte-identical to
upstream (corpus + raise parity tests). Pin drop_thinking=False on every
V3.2 surface and register {tool, user}; pure tool-loop histories render
byte-identically under either drop mode, so existing {tool} configs are
unaffected.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

guapisolo added a commit that referenced this pull request Jul 25, 2026
…server v2

The v5 multi-lineage design lands as an OPT-IN second implementation
instead of replacing v1: --use-session-server grows an optional value
(bare flag or "v1" = the untouched linear server, "v2" = tree serving),
and the whole tree stack lives in miles/rollout/session/v2/. The v1
modules stay byte-identical to their pre-branch state.

v2 package (carried from the v5 line, imports adjusted only):
- session_state: always-branch serving over the forest — deepest attach
  point, suffix becomes the branch delta, non-extensions grow siblings or
  new roots; retry semantics moves entirely to the samples-op picker.
  Branch suffixes may carry client assistants (compaction); snapshot
  splice with zero-inheritance-root fallback on canonical-prefix
  divergence. Truncated-path extension is 409 (TruncatedGenerationError,
  defined here — v1 never raises it). --session-strict-append-only is the
  single-chain guard: branch shapes fail loud with attach diagnostics.
- assembly: per-leaf fold (compute -> truncate -> fold), default pick
  (temporal-supersession retry trim; roots never trimmed; non-retry-shaped
  trees 422) and default merge (exactly-once completion masking over the
  surviving set, rewards keyed by response id, fold < agent < server
  metadata layering); both replaceable via --session-sample-picker-path /
  --session-merge-function-path (sync-only, loaded in-process).
- core: v2 twin of SessionCore reusing v1's HTTP plumbing unchanged.

Shared seams, each inert for v1 by construction:
- sessions.py dispatches registry/core on the flag and forwards the
  collect body's "metadata" (agent semantic layer) only under v2.
- codec: encode/decode parameterized by a fields tuple; v1 default keeps
  the wire byte-identical, v2 adds reward + per-sample metadata
  (COMPUTED_FIELDS_V2) with conditional overlay semantics on decode.
- arguments: flag upgrade (nargs="?"), unknown values rejected, and the
  three v2 flags require --use-session-server v2.

Tests: v2 HTTP matrix (tree pins incl. deep/root divergence branching,
strict guard, truncation + compaction), session_state unit matrix, and
the samples-op suite (golden, multi-leaf trim/masking/rewards, hook
lanes) — all against a v2-flagged server; the restored v1 suites and the
byte-exact A-list (git diff against the pre-branch base is empty) pin
that the default path did not move.

Rebased onto the tito/session PR stack (#1628/#1777/#1778 +
#1759/#1760), with the stack's landed shape taken as authoritative:

- supports_midpath_assistant_rerender (True on the base class) moves in
  here with its consumer, the v2 suffix gate. No family sets it False
  anymore: the vendored DeepSeek-V3.2 encoder renders
  position-independently under drop_thinking=False, so the gate text no
  longer names it; a stub family pins the fail-loud branch.
- v2 serving fills SessionRecord.request_timestamp like v1, so folded
  samples keep their lifecycle req_ts across the wire.
- codec: COMPUTED_FIELDS_V2 is re-expressed over the ValueSpec table
  (SAMPLES_VALUE_SPEC_V2 superset), same wire semantics.
- v2 unit tests pin allowed_append_roles=["tool"] explicitly; the ctor
  default is tool+user since the append-roles PR.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Keep the upstream and vendored encoder modules at test module scope so the parity helpers and exception checks share the reviewed dependencies without changing behavior.
@guapisolo
guapisolo force-pushed the feat/tito-default-append-tool-user branch from eb9a023 to 0ec4229 Compare July 25, 2026 08:13
@guapisolo
guapisolo force-pushed the feat/tito-append-roles-harness-only branch from c171707 to bdcd0af Compare July 25, 2026 08:14
@guapisolo
guapisolo force-pushed the feat/tito-default-append-tool-user branch from 0ec4229 to 20b701d Compare July 25, 2026 08:39
@guapisolo
guapisolo force-pushed the feat/tito-append-roles-harness-only branch from bdcd0af to f437d54 Compare July 25, 2026 08:39
Keep the Miles-owned V3.2 encoder copy for drop_thinking while restoring V4 to SGLang encoding_dsv4. Narrow the pre-commit exclusion and parity-test import to the remaining V3.2 vendoring.
@guapisolo
guapisolo force-pushed the feat/tito-default-append-tool-user branch from 20b701d to 55a21c5 Compare July 25, 2026 18:40
@guapisolo
guapisolo force-pushed the feat/tito-append-roles-harness-only branch from f437d54 to 2ecc262 Compare July 25, 2026 18:40
@guapisolo
guapisolo force-pushed the feat/tito-default-append-tool-user branch from 55a21c5 to 7075ef8 Compare July 25, 2026 21:56
@guapisolo
guapisolo force-pushed the feat/tito-append-roles-harness-only branch from 2ecc262 to 6b5397d Compare July 25, 2026 21:56
@guapisolo
guapisolo force-pushed the feat/tito-append-roles-harness-only branch from 6b5397d to a681181 Compare July 25, 2026 23:07
@guapisolo
guapisolo force-pushed the feat/tito-default-append-tool-user branch 2 times, most recently from 673151c to d1d4845 Compare July 25, 2026 23:36
@guapisolo
guapisolo force-pushed the feat/tito-default-append-tool-user branch 2 times, most recently from c7934eb to 5b9f935 Compare July 27, 2026 04:13
@guapisolo
guapisolo requested a review from yushengsu-thu as a code owner July 27, 2026 04:13
@guapisolo
guapisolo force-pushed the feat/tito-default-append-tool-user branch 7 times, most recently from a7da070 to f2ae6e8 Compare July 27, 2026 06:15
@guapisolo

Copy link
Copy Markdown
Collaborator Author

Superseded by #1777: the flag-as-harness-gate design evolved into binding append roles to fixed templates (f230f07) and removing --tito-allowed-append-roles outright with FIXED_TEMPLATE-derived verify surfaces (19c3170). The still-valid test coverage from this branch (restricted-family schedule derivation, assistant-input rollback/regenerate, injected-assistants-are-not-generated-checkpoints) has been ported into those commits on #1777.

@guapisolo guapisolo closed this Jul 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant