Skip to content

Fix incomplete streams counted as successful in shared-prefix load tests - #889

Merged
ashhart merged 1 commit into
ashhart:mainfrom
alenperic:fix/incomplete-load-streams
Oct 16, 2026
Merged

ashhart merged 1 commit into
ashhart:mainfrom
alenperic:fix/incomplete-load-streams

Conversation

@alenperic

Copy link
Copy Markdown
Contributor

What this changes

Fixes tools/shared_prefix_load.py counting streams that end without [DONE] as successful requests, which could inflate reported throughput. The client now stops at [DONE], records premature EOF as incomplete_stream, and preserves existing server or transport errors.

Adds six regression tests covering complete and truncated streams, termination at [DONE], error preservation, and exclusion of incomplete requests from successful token totals.

Receipt

  • Environment: Candidate 0e871b39a30878df1f3ef035167e5e1bb2fea6db, based on c7137bb31a7dad924fb541a7068422127b9a94b2. Independent verification on Linux x86_64 with Python 3.12.14. Standard-library tests; no model, checkpoint, GPU, MLX, or PyTorch required.
  • Exactness: n/a. Changes are limited to benchmark-client stream handling and tests; native token generation, draft verification, and model arithmetic are unchanged.
  • Decode speed before and after: n/a. No inference performance claim. The change corrects which requests contribute to benchmark results.
  • Prompt speed before and after: n/a. The change cannot reach server-side prompt processing.
  • Tests run:
    • New regression suite against the parent implementation: six test methods; three passed and three failed, with four recorded failures because the EOF test includes two failing subtests. No errors or skips.
    • Candidate code: python3 -m unittest discover -s tools/tests -v — 23 passed, 0 failed, 0 errors, 0 skipped.
  • Lean checker: 1,375 files checked; baseline 527 findings, candidate 527 findings; zero added or removed findings. Both runs exit 1 due to inherited findings.
  • Not run, and why: Native host/golden suites, Metal M1–M5 and CUDA model checks, and live inference benchmarks were not run because this change is confined to Python client bookkeeping. macOS execution was not independently verified.

Checklist

  • Lane rounds and serial/draft execution paths are unchanged.
  • Drafted/plain, resumed/fresh, and concurrent/solo model exactness checks — not run; native inference is unchanged.
  • No precision or reduction-order changes.
  • Prompt performance against the last release — not measured; server-side prompt processing is unchanged.
  • Tested and untested platforms are identified above.
  • Regression cases reproduce the bug before the change and pass afterward. Tests use only the standard library and require no optional dependency skips.
  • Added comments/docstrings are single-line; no measurements or history were added to source.
  • Lean checker run; baseline and candidate findings recorded, with zero new findings.
  • The two changed files and outgoing commit metadata were reviewed; author and committer use GitHub noreply.

@ashhart
ashhart merged commit 10bc29b into ashhart:main Oct 16, 2026
ashhart added a commit that referenced this pull request Oct 16, 2026
…VIEW names the refused GLM pair requests rightly

One line each for #881, #882, #883, #889, #890, #709, #848, #892, #867, #868, #871 and #873, crediting their
authors. ZIG-PREVIEW said a two-Mac GLM pair refuses "temperature-1" requests; it refuses any temperature above 0
(backend.zig and engine.zig both test temperature > 0).
@ashhart

ashhart commented Oct 16, 2026

Copy link
Copy Markdown
Owner

Thank you, @alenperic, for excluding incomplete streams from successful benchmark results.

#889 is included in 1.0.12 as merge 10bc29b, at 0e871b3. tools/shared_prefix_load.py counts an EOF without data: [DONE] as failed and excludes its tokens from successful throughput and first-token summaries, while preserving server and transport error messages.

Its tests and the other benchmark client tests pass under Python 3.12 on macOS and Linux.

Posted by Ash's AI Assistant Synthena

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.

2 participants