Repository navigation
docs: qualify Sarathi chunked-prefill budget case - #277
Conversation
Retain mixed-batch accounting, chunk overhead and the paper's TTFT/TBT tradeoffs in one attributed candidate under the existing search-policy owner. Keep empirical validation not_run and raw source reuse rights UNKNOWN. Agent-assisted-by: Codex / gpt-6.1-sol [effort=max] (author) Signed-off-by: Dong1017 <55723162+Dong1017@users.noreply.github.com>
Refresh the native catalog from the committed canonical source without changing source state, empirical status or attribution. Agent-assisted-by: Codex / gpt-6.1-sol [effort=max] (author) Signed-off-by: Dong1017 <55723162+Dong1017@users.noreply.github.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Skill evaluationAutomatic execution / cumulative PR budget: Advisory: obtain missing evidenceRetained evidence availability: planned / unexecuted. No approval or merge authorization is issued.
PLANNED: execution is not authorized by knowing this hash. Selected comparisons: 1 · Estimated task calls: 2. Applicable only while these revisions and inputs match the current PR. Plan bindings, budget, frozen exam and advisory detailsUnchanged-correct is valid neutral evidence, not incremental benefit. Per-case outcomes (discovery excluded; regressions and gaps first): []Showing at most 20 of 0 pairs; the evidence link contains all outcomes, including omitted regressions/gaps. Plan SHA-256: Model/settings:
Frozen exam hashes: Provider/settings: Uncovered / unsupported paths (82)
|
Refresh the native coverage projection for the committed Sarathi case and its provenance readset without changing canonical source content. Agent-assisted-by: Codex / gpt-6.1-sol [effort=max] (author) Signed-off-by: Dong1017 <55723162+Dong1017@users.noreply.github.com>
Dong1017
left a comment
There was a problem hiding this comment.
LGTM for the bounded source-only candidate slice at e3db289c; I found no blocking content or packaging issue.
- The decode → partial-prefill → new-request accounting, chunk overhead, and Table 4 TTFT/TBT tradeoff match the fixed OSDI edition. The case avoids claiming a universal budget or current-engine support.
- Current-head Ready checks and the latest declaration status passed. Skill Eval is planned/unexecuted; my recommendation covers attributed source guidance with
candidate / not_runpreserved, not empirical skill or model benefit. - Integration remains with the existing author, @Dong1017. After another of PRs #277–#279 lands, preserve all canonical additions, commit the combined source snapshot, regenerate the native catalog and coverage view, then verify the new head. In particular, #277/#278 share the search-policy owner; choosing one generated side would lose the combined projection.
Evidence and scope
- Reviewed head:
e3db289c42d6d37374755a05f28b1107da4f73ae. - API base:
ffb77f9a168ba1cb22f7ba261f289d24dbb6698f; assessed main:3ece458ceaa92c5dad8d8e0b1e782a6961400b96. - Hosted tested merge:
49947440bfa8ffb7874d7665f603af301fc1ed93, with verified parents main + reviewed head. Ready run: Python 3.10/3.11 full-check and installed-package jobs, plus KV task qualification succeeded. The 3.10 job reports 3168 passed, 13 skipped and 284 passing subtests. Latest declaration run succeeds; earlier failed runs remain historical. Declaration compliance does not verify actual author settings. - Scope: Sarathi case/record, owner procedure/metadata, both matrices, case-count assertion and all generated catalog/coverage changes. Canonical source snapshot
84207eaaccb7987612a115f80a85c61a4c68cd56is reachable; only projections change between it and this head. Changed owner/resource hashes match the catalog; sidecar fields are preserved, with the reader's expected name/description fields added. Official paper bytes match the recorded PDF SHA-256. Source locations and historical results were checked; I did not reproduce the paper's experiments. - Skill Eval plan: 1 selected comparison(s), 92 uncovered/unsupported changes; execution skipped with unavailable/expired trusted configuration/pricing. These gaps are explicitly retained for this source-only review, with no efficacy acceptance or candidate promotion. Earlier incomplete bot reviews are not approvals.
- Integration scope is conditional on the peer versions: #277
e3db289c42d6d37374755a05f28b1107da4f73ae; #2787ef53189343c4bf04fc9e31b4d80f9644482cdfc; #279cf0b7022761fa83361f986e3b37c2031ad33fd8d. Their combination has not been tested. Main's intervening activation change does not alter these knowledge inputs; each hosted merge already includes that main revision. - This review task made no implementation changes and ran no local pytest, model, GPU or paid evaluation. The posting account is also the PR author account, so this is a COMMENTED review, not GitHub APPROVED. It grants no merge, execution or whole-tracker acceptance authority.
Preserve the reviewed canonical case and all current-main files before regenerating native catalog and coverage projections. Agent-assisted-by: Codex / gpt-6.1-sol [effort=max] (author) Signed-off-by: Dong1017 <55723162+Dong1017@users.noreply.github.com>
Run both native generators against the committed combined source tree. Retain all skill metadata, source hashes and candidate validation limits. Agent-assisted-by: Codex / gpt-6.1-sol [effort=max] (author) Signed-off-by: Dong1017 <55723162+Dong1017@users.noreply.github.com>
Governing Issue and delivery slice
Refs #91.
This PR adds one original attributed
case.sarathi-chunk-budgetunder the existingmethods/optimization-search-policyowner, grounded in the fixed official OSDI 2024 edition. It covers decode/partial-prefill/new-request accounting, chunk overhead and Table 4's actual TTFT/TBT tradeoffs. Admission, preemption and calibration remain with their existing owners.The identity and central mechanism overlap closed-unmerged #156. This independently written case does not reopen that PR, recover the missing historical archive, or claim novelty over unavailable supplier synthesis. Raw PDF/text/images, reproduced figures, long quotations and prior proposal prose are excluded. Raw source redistribution/adaptation rights remain
UNKNOWN; the case remainscandidatewith empiricalnot_run.Changed artifacts and evidence
The case document and source record are linked from the existing owner and both matrices. The native inventory has 77 skills and 7 cases. The integrated base is
ecca7e9136d962747f42860f58e332f45b0aa358. Normal canonical merge M isc5e13944c06a8a03ab7015a8933981a983e42186; native projection commit G and final head are0750bc61532b6b67e7385557d572e2f4e1cd2203. The merge preserves all earlier contribution commits and current-main files, with canonical source, metadata and matrix bytes unchanged from the previously reviewed public heade3db289c42d6d37374755a05f28b1107da4f73ae.Both native generators ran from M. All 77 catalog records cite M and retain their existing source hashes and skill metadata. Native coverage regeneration returns the same view bytes: the merged main adds no registered-skill, matrix or protocol readset inputs to that view.
Checks and dependencies
--check, canonical/current-main preservation andgit diff --check.-I -Sbootstrap over existing dependency paths. Earlier optional broad local suite/build jobs had no completion receipt; no local wheel or installed-package result is claimed, and those broad jobs were not repeated.Remaining work and Issue closure
The broader #91 queue and original-archive recovery remain separate. Goal acceptance belongs to @Dong1017. Merge readiness requires fresh review and all applicable checks at the final head; Issue completion and closure are not claimed by this contribution.
Knowledge authorship declaration