(2/2) refactor(session): assemble training samples on the session server; records never leave it - #1759
Merged
Merged
Conversation
guapisolo
requested review from
Shi-Dong,
Zhichenzzz,
fzyzcjy,
jybsuper,
maocheng23 and
yueming-yuan
as code owners
July 21, 2026 23:09
Contributor
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
guapisolo
commented
Jul 22, 2026
| @@ -0,0 +1,222 @@ | |||
| """Black-box sample packing for the `POST /sessions/{id}/samples` reply; only the | |||
Collaborator
Author
There was a problem hiding this comment.
This is a vibed file, but verified by e2e CIs. No tito mismatch rate regression.
guapisolo
force-pushed
the
refactor/session-samples-move
branch
from
July 23, 2026 19:40
0d64438 to
8c55e0e
Compare
guapisolo
force-pushed
the
refactor/session-samples-op
branch
from
July 23, 2026 19:40
95a27be to
8fdc5d5
Compare
guapisolo
marked this pull request as draft
July 24, 2026 20:39
guapisolo
force-pushed
the
refactor/session-samples-op
branch
from
July 24, 2026 20:59
8fdc5d5 to
a70ff78
Compare
guapisolo
force-pushed
the
refactor/session-samples-move
branch
from
July 24, 2026 21:13
8c55e0e to
b82b089
Compare
guapisolo
force-pushed
the
refactor/session-samples-op
branch
from
July 24, 2026 21:24
a70ff78 to
c95ee80
Compare
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>
4 tasks
guapisolo
force-pushed
the
refactor/session-samples-op
branch
from
July 28, 2026 07:47
c95ee80 to
98b4e42
Compare
guapisolo
marked this pull request as ready for review
July 28, 2026 18:12
guapisolo
force-pushed
the
refactor/session-samples-op
branch
from
July 28, 2026 18:56
98b4e42 to
8875bc6
Compare
guapisolo
changed the base branch from
main
to
refactor/remove-generate-multi-samples
July 28, 2026 18:58
guapisolo
force-pushed
the
refactor/session-samples-op
branch
from
July 29, 2026 00:27
d2ac3c5 to
51b62ac
Compare
Shi-Dong
reviewed
Jul 29, 2026
Comment on lines
-59
to
-64
| except asyncio.TimeoutError: | ||
| logger.error( | ||
| f"Timed out waiting for session {self.session_id} records after {_SESSION_REQUEST_TIMEOUT}s " | ||
| f"(likely stale HTTP keepalive connection). Returning empty records." | ||
| ) | ||
| # Still attempt to clean up the session. |
Contributor
There was a problem hiding this comment.
Something I don't understand: what if TimeoutError is raised here? Wouldn't the error be propagated out and crash the rollout group?
Collaborator
Author
There was a problem hiding this comment.
let me carefully check this.
…ds never leave it
POST /sessions/{session_id}/samples lets the owning session-server instance run record assembly, turn-boundary truncation, and the unconditional linear merge before returning one safetensors payload. SAMPLES_VALUE_SPEC defines the tensor/json wire fields and pinned dtypes; the driver overlays them onto a deepcopy of its input Sample. Server-produced metadata crosses as a delta, so lifecycle and debug messages survive while input-only task metadata remains local.
The training path no longer downloads full session records to the rollout driver. OpenAIEndpointTracer performs one non-retrying binary POST, raises on timeout and non-2xx replies, and still attempts DELETE on every path. agentic_tool_call keeps its stable list[Sample] contract, maps empty reasons to a singleton ABORTED result, then applies agent and session metadata in order.
Tests cover merged and truncated golden samples, metadata authority and isolation, malformed safetensors payloads, 422/404/empty-reason/route-order behavior, client failure semantics, and the HTTP-only generate happy path.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Rename the codec entry points to distinguish wire encoding from driver-side Sample assembly. Simplify their local documentation while preserving the payload and merge behavior.
Keep the samples collection and assembly documentation focused on stable behavior without retaining implementation-history detail.
guapisolo
force-pushed
the
refactor/session-samples-op
branch
from
July 29, 2026 21:17
51b62ac to
21b59dd
Compare
Map collect_samples timeouts to a copied ABORTED sample at the agentic generate boundary so one transient collection timeout does not fail the rollout group. Keep OpenAIEndpointTracer fail-loud and continue propagating non-timeout collection errors.
Document that collect_samples leaves request timeouts fail-loud after the session cleanup attempt so the agentic generate boundary can map them to an ABORTED sample.
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.
Summary
Assemble agentic training samples on the owning session server over HTTP.
Motivation
The rollout driver previously downloaded every session record before computing, truncating, and merging the training sample. Because each record contains the full trajectory prefix, that transfer grows quadratically with the number of turns and makes the driver serialize, transfer, and parse data whose only consumer is sample assembly.
This PR establishes HTTP as the complete baseline transport. It intentionally adds no Mooncake dependency or runtime assumption; a later PR can add Mooncake as an optional optimization without changing the assembly contract.
Before / After
SAMPLES_VALUE_SPEC.Sample, so input-only fields remain local.OpenAIEndpointTracer.collect_samplesperforms one non-retrying binary HTTP POST.Behavior Preservation
Samplevalues, including truncation.input < server < agent < session.no_recordsandall_truncatedremain singletonABORTEDresults.Verification
Review Focus
SessionCore.collect_samples: assembly stays synchronous on the event loop so its lock-free read sees one coherent records list.SAMPLES_VALUE_SPEC: it is the sole field allowlist plus dtype/codec contract for the wire.