Skip to content

Add provider smoke harness (openvino/qnn/migraphx/coreml/dnnl EP smokes) - #391

Merged
tonythethompson merged 5 commits into
mainfrom
vibe/provider-smoke-harness-dfc30d
Oct 6, 2026
Merged

tonythethompson merged 5 commits into
mainfrom
vibe/provider-smoke-harness-dfc30d

Conversation

@tonythethompson

@tonythethompson tonythethompson commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Adds tools/provider-smoke/ — a dependency-light Python harness that answers "does execution provider X load, take nodes, and run on this host" in seconds, complementing the .NET provider-matrix scenario (which measures shipped pipeline stages end-to-end)
  • Covers cpu, openvino, qnn (auto-registers the wheel's provider library + HTP backend path), migraphx, dnnl, coreml tokens; explicit never-fabricated status semantics: ok / fallback (EP loaded, all nodes on CPU) / fail (with error) / absent (platform prerequisite missing — e.g. CoreML on non-macOS)
  • Records session-create time, best/mean run ms per provider; refuses to guess feeds for models with non-batch dynamic dims (requires --feed npz, avoiding misleading smoke results)
  • README documents per-provider wheel variants, hardware prerequisites, and initial sandbox evidence (linked to Design spec: benchmark-first CPU text refinement (llama.cpp tier conditional) #385/Add tts-bench command: Kokoro TTS benchmarking for DubBench #388)

Verification

  • Harness run against whisper-base encoder in three environments: openvino wheel (cpu ok, openvino ok), qnn wheel (qnn fallback — clean load, no Hexagon device, matches repo QNN readiness-probe semantics), migraphx wheel (migraphx fallback — missing ROCm libmigraphx)
  • python -m py_compile clean

Initial evidence summary (GPU/NPU-less Intel sandbox)

  • openvino EP works on the CPU plugin but is ~11% slower than plain CPU EP on transformer encoders and rejects Kokoro's dynamic-rank STFT graph
  • qnn EP loads cleanly via register_execution_provider_library and falls back to CPU without Hexagon; the wheel ships no x86 QnnCpu backend
  • migraphx wheel exists for Linux x86_64 but requires the AMD ROCm stack (absent); CoreML wheel is macOS-only
  • DNNL has no PyPI wheel — it requires the repo's own native build (tools/onnxruntime-dnnl/)

Review in cubic

mistral-vibe and others added 4 commits October 6, 2026 14:57
…ense gates

Co-authored-by: tonythethompson <tonythethompson@users.noreply.github.com>
…trained CPU

Co-authored-by: tonythethompson <tonythethompson@users.noreply.github.com>
Co-authored-by: tonythethompson <tonythethompson@users.noreply.github.com>
Co-authored-by: tonythethompson <tonythethompson@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 6, 2026 16:40
@cursor

cursor Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • queue - adds this PR to the back of the merge queue
  • fast - for urgent changes, fast-track this PR to the front of the merge queue

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

@coderabbitai

coderabbitai Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: a2a851ff-d228-485f-85f3-2df3c47527d7
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@amazon-q-developer amazon-q-developer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

This PR adds a Python-based provider smoke test harness and TTS benchmarking infrastructure. The smoke harness tests execution provider loading and basic inference, while the TTS benchmark framework measures Kokoro TTS performance.

Critical Issues Found (3)

Three defects were identified that could cause crashes or incorrect behavior:

  1. Python crash risk in qnn_backend_path(): Missing ImportError handling will crash when onnxruntime_qnn is not installed
  2. Unused import: The ctypes module is imported but never used
  3. Python crash risk in bench_run(): Function will crash with ValueError when repeats parameter is 0

The C# code (TtsEvalRunner.cs and related files) is well-structured with proper error handling and validation. The logic correctly validates that repeatRuns >= 1 at the argument parsing level, preventing invalid values from reaching the benchmark execution.

Recommendation

Address the three Python defects before merging. The C# implementation is production-ready.


You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.

Comment thread tools/provider-smoke/smoke_execution_providers.py
Comment thread tools/provider-smoke/smoke_execution_providers.py
Comment thread tools/provider-smoke/smoke_execution_providers.py
@tonythethompson
tonythethompson marked this pull request as ready for review October 6, 2026 16:42
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-06T16:44:43.887773Z e3d1f0a Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@tonythethompson tonythethompson left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the provider-smoke commit (e3d1f0a) only. This branch is stacked on #388, so the tts-bench files are covered by the threads there. Four inline findings below; I didn't repeat Amazon Q's three (unguarded onnxruntime_qnn import, unused ctypes/DEFAULT_DYNAMIC_DIM, --repeat-runs 0).

Comment thread tools/provider-smoke/smoke_execution_providers.py Outdated
Comment thread tools/provider-smoke/smoke_execution_providers.py Outdated
Comment thread tools/provider-smoke/smoke_execution_providers.py Outdated
Comment thread tools/provider-smoke/smoke_execution_providers.py
@qodo-code-review

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Add execution-provider smoke harness and TTS benchmark command

✨ Enhancement 🧪 Tests 📝 Documentation 🕐 40+ Minutes

Grey Divider

AI Description

• Add lightweight ONNX Runtime smokes to check provider loading, inference, fallback, and timing.
Diagram

graph TD
  A["Smoke CLI"] --> B["Validated feeds"] --> C["Provider setup"] --> D["ORT session"] --> E[("Smoke JSON")]
  F["TTS bench"] --> G["Headless TTS host"] --> H[("Results JSONL")]
  A -. "complements" .-> F
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Extend the existing .NET provider matrix
  • ➕ One benchmark entry point and reporting stack.
  • ➖ Requires shipped pipeline setup for a basic provider check.
  • ➖ Less convenient for testing separate Python ONNX Runtime wheel variants.

Recommendation: Keep the lightweight Python smoke separate from the end-to-end .NET matrix, and keep TTS measurement in the existing .NET benchmark host. They answer different readiness and performance questions without adding a second production routing path.

Files changed (7) +1030 / -0

Enhancement (4) +666 / -0
BenchmarkConsole.csList the TTS benchmark command in CLI help +2/-0

List the TTS benchmark command in CLI help

• Adds tts-bench usage and help entries to the benchmark console.

src/Trackdub.Benchmarks/BenchmarkConsole.cs

Program.csDispatch the tts-bench command +28/-0

Dispatch the tts-bench command

• Registers the command and routes parsed options to the TTS evaluation runner, including help and argument-error handling.

src/Trackdub.Benchmarks/Program.cs

TtsEvalRunner.csBenchmark TTS synthesis jobs through the headless host +408/-0

Benchmark TTS synthesis jobs through the headless host

• Reads JSONL jobs, selects a registered TTS engine, and writes per-job results with wall time, real-time factor, working set, and provider details. Supports warmups, repeats, provider pinning, cancellation, and recording individual job failures.

src/Trackdub.Benchmarks/TtsEvalRunner.cs

smoke_execution_providers.pyAdd a standalone ONNX Runtime provider smoke harness +228/-0

Add a standalone ONNX Runtime provider smoke harness

• Builds or loads model feeds, configures requested providers, runs and times inference, and emits a JSON report. Includes QNN library registration, a CoreML platform check, and distinct success, fallback, failure, and absence statuses.

tools/provider-smoke/smoke_execution_providers.py

Tests (1) +206 / -0
TtsEvalRunnerTests.csTest TTS benchmark parsing and job execution +206/-0

Test TTS benchmark parsing and job execution

• Covers JSONL and CLI validation, command dispatch, result fields, provider preferences, failure continuation, and cancellation using fake dependencies.

tests/Trackdub.Benchmarks.Tests/TtsEvalRunnerTests.cs

Documentation (2) +158 / -0
design-piper-cpu-tts-tier.mdPropose an evidence-gated Piper CPU fallback +87/-0

Propose an evidence-gated Piper CPU fallback

• Defines a benchmark-first decision process for whether Kokoro needs a Piper fallback. Records preliminary CPU results and makes planner routing, phonemizer packaging, and licensing explicit gates before implementation.

docs/specs/design-piper-cpu-tts-tier.md

README.mdDocument provider smoke usage and host prerequisites +71/-0

Document provider smoke usage and host prerequisites

• Explains statuses, feed and timing options, provider-specific wheel requirements, QNN registration, and initial hardware-limited evidence.

tools/provider-smoke/README.md

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e3d1f0a71e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread tools/provider-smoke/smoke_execution_providers.py Outdated
Comment thread src/Trackdub.Benchmarks/TtsEvalRunner.cs
Comment thread src/Trackdub.Benchmarks/TtsEvalRunner.cs
Comment thread src/Trackdub.Benchmarks/BenchmarkConsole.cs
@qodo-code-review

qodo-code-review Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

1. CPU-only runs appear to use an accelerator ✗ Dismissed
Description
smoke marks a run ok when the requested provider appears in sess.get_providers(), which does
not establish that any graph nodes ran on it. When the provider accepts a session but assigns every
node to CPU, the report says ok instead of the promised fallback.
Code

tools/provider-smoke/smoke_execution_providers.py[R180-183]

+        if any(p == requested for p in providers_in_use):
+            entry["status"] = "ok"
+        else:
+            entry["status"] = "fallback"
Relevance

●●● Strong

Session provider presence does not prove graph execution; this contradicts the documented fallback
status contract.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The implementation bases status solely on the session provider list, while its stated status
contract distinguishes runs by where nodes execute.

tools/provider-smoke/smoke_execution_providers.py[176-184]
tools/provider-smoke/README.md[11-16]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The smoke result treats session provider membership as proof that the requested provider executed nodes.
## Fix Focus Areas
- tools/provider-smoke/smoke_execution_providers.py[176-184]
## Recommended Fix
Inspect actual node placement, for example through ONNX Runtime profiling, and classify CPU-only execution as `fallback`. Preserve `ok` only when the requested provider executes at least one node.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Some registered TTS engines always fail ✗ Dismissed
Description
RunJobAsync invokes the adapter's unplanned SynthesizeAsync overload instead of the routed TTS
entry point. Qwen3-TTS, Chatterbox, and CosyVoice explicitly reject that overload, so selecting any
of those registered engine families fails every job before synthesis.
Code

src/Trackdub.Benchmarks/TtsEvalRunner.cs[R328-329]

+                var warmClock = Stopwatch.StartNew();
+                warm = await engine.SynthesizeAsync(request, cancellationToken).ConfigureAwait(false);
Relevance

●●● Strong

The benchmark’s stated planner-routing intent conflicts with directly invoking an overload rejected
by registered engines.

PR-#257

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The runner calls the unplanned overload directly; three registered engines throw from that overload,
whereas the routed engine selects a plan and calls the planned overload.

src/Trackdub.Benchmarks/TtsEvalRunner.cs[220-235]
src/Trackdub.Benchmarks/TtsEvalRunner.cs[324-340]
src/Trackdub.Inference.Onnx/Qwen3Tts/Qwen3TtsEngine.cs[41-51]
src/Trackdub.Inference.Onnx/Chatterbox/ChatterboxVoiceCloneTtsEngine.cs[54-57]
src/Trackdub.Inference.Onnx/CosyVoice/CosyVoiceTtsEngine.cs[30-35]
src/Trackdub.Inference.Onnx/Runtime/Routing/RoutedTtsEngine.cs[46-60]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The benchmark calls an adapter overload that several registered TTS engines intentionally reject.
## Fix Focus Areas
- src/Trackdub.Benchmarks/TtsEvalRunner.cs[220-235]
- src/Trackdub.Benchmarks/TtsEvalRunner.cs[324-340]
## Recommended Fix
Route benchmark synthesis through the existing TTS planner and invoke the selected adapter's planned overload. Account for engines that require additional request inputs before advertising them as benchmarkable.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

3. Zero-repeat jobs report successful timing ✗ Dismissed
Description
ReadJobs does not validate per-job RepeatRuns or WarmupRuns overrides before RunJobAsync
uses them. A job with repeat_runs: 0 reports success with zero measured time, while a negative
value aborts the benchmark at list construction instead of recording a failed job.
Code

src/Trackdub.Benchmarks/TtsEvalRunner.cs[R320-321]

+        int warmupRuns = job.WarmupRuns ?? options.WarmupRuns;
+        int repeatRuns = job.RepeatRuns ?? options.RepeatRuns;
Relevance

●●● Strong

Per-job overrides bypass CLI validation; zero and negative repeat counts create clear correctness
failures.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Job validation checks only required identity fields; zero repeats leave walls empty yet reach the
successful result, and negative repeats reach new List<double>(repeatRuns).

src/Trackdub.Benchmarks/TtsEvalRunner.cs[163-175]
src/Trackdub.Benchmarks/TtsEvalRunner.cs[320-354]
src/Trackdub.Benchmarks/TtsEvalRunner.cs[89-101]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Per-job run-count overrides bypass the CLI's validation and can yield fabricated timing or abort the run.
## Fix Focus Areas
- src/Trackdub.Benchmarks/TtsEvalRunner.cs[163-175]
- src/Trackdub.Benchmarks/TtsEvalRunner.cs[320-350]
## Recommended Fix
Reject negative warmup counts and nonpositive repeat counts while reading jobs, with the job line number in the error. Add tests for zero and negative overrides.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


4. Multiple provider options split into tests ✗ Dismissed
Description
smoke parses comma-separated options after a provider's colon, but main splits the entire
--providers argument on commas first. Supplying two options to one provider instead creates a
second, unintended provider smoke and passes only the first option to the intended one.
Code

tools/provider-smoke/smoke_execution_providers.py[213]

+    for spec in [s.strip() for s in args.providers.split(",") if s.strip()]:
Relevance

●●● Strong

Outer comma splitting makes the documented comma-separated provider options unusable, a
deterministic parser bug.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The inner parser expects comma-separated key-value options, while the outer loop splits on those
same commas before calling it.

tools/provider-smoke/smoke_execution_providers.py[151-158]
tools/provider-smoke/smoke_execution_providers.py[213-219]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Commas serve as both provider and option separators, so a provider cannot receive multiple options.
## Fix Focus Areas
- tools/provider-smoke/smoke_execution_providers.py[151-158]
- tools/provider-smoke/smoke_execution_providers.py[213-219]
## Recommended Fix
Choose distinct delimiters or a structured argument for provider entries and their options. Parse complete entries without splitting their option lists, and document and test the resulting syntax.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


5. Valid model aliases cannot be benchmarked ✗ Dismissed
Description
RunAsync compares --model directly with each adapter's EngineFamily instead of resolving the
supplied model alias through the planner. Manifest aliases such as qwen3-tts-0.6b therefore
produce “No TTS engine” even though they belong to a registered family.
Code

src/Trackdub.Benchmarks/TtsEvalRunner.cs[R221-223]

+            ITtsEngineAdapter? engine = scope.ServiceProvider
+                .GetServices<ITtsEngineAdapter>()
+                .FirstOrDefault(e => string.Equals(e.EngineFamily, options.Model, StringComparison.OrdinalIgnoreCase));
Relevance

●●● Strong

Accepted history favors validating model aliases against runtime planning rather than direct family
matching.

PR-#257

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The manifest lists aliases distinct from their engine family; the runner's family comparison rejects
them, while existing routing plans from the alias.

src/Trackdub.Benchmarks/TtsEvalRunner.cs[220-227]
src/Trackdub.Inference/Runtime/ModelManifest/bundled-models.manifest.json[4117-4120]
src/Trackdub.Inference/Runtime/ModelManifest/bundled-models.manifest.json[4141-4145]
src/Trackdub.Inference.Onnx/Runtime/Routing/RoutedTtsEngine.cs[91-116]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The model argument is treated as an engine-family name, rejecting valid manifest aliases.
## Fix Focus Areas
- src/Trackdub.Benchmarks/TtsEvalRunner.cs[220-228]
- src/Trackdub.Benchmarks/TtsEvalRunner.cs[311-319]
## Recommended Fix
Pass `--model` as a preferred model alias to the existing routed TTS path, then use the planner-selected engine family rather than comparing the alias with adapter family names.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View medium (1)
6. Integer-input models cannot be smoked ✗ Dismissed
Description
build_feed casts every supplied tensor to float32, even when the model declares an integer
input. A valid feed for a model such as Kokoro, whose input_ids tensor is int64, is changed to
the wrong type before inference and fails for every requested provider.
Code

tools/provider-smoke/smoke_execution_providers.py[R105-107]

+    if feed_path:
+        data = dict(np.load(feed_path))
+        return {k: np.asarray(v, dtype=np.float32) for k, v in data.items()}
Relevance

●●● Strong

Unconditionally coercing explicit feeds to float32 breaks valid integer ONNX inputs, a deterministic
correctness bug.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The explicit-feed branch unconditionally converts arrays to float32, but the shipped Kokoro
inference path supplies an int64 tensor for input_ids.

tools/provider-smoke/smoke_execution_providers.py[104-108]
src/Trackdub.Inference.Onnx/Kokoro/KokoroTtsEngine.cs[278-292]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The feed loader converts all supplied tensors to float32, preventing integer-input models from running.
## Fix Focus Areas
- tools/provider-smoke/smoke_execution_providers.py[104-133]
## Recommended Fix
Preserve the dtypes stored in explicit NPZ feeds. For generated feeds, inspect each model input's declared type and generate a compatible tensor or require an explicit feed when generation is unsafe.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
✅ Compliance rules (platform): 4 rules
✅ Cross-repo context — repo relationships
✅ REVIEW.md
Review mode: Auto: ⚖️ Balanced: Downgraded extended -> standard: change is below the extended eligibility bar (hunks 8/18, lines 1030/200; both must reach the floor). Router rationale: Substantial new Python and C# logic spans independent provider and benchmarking paths.

Grey Divider

Tip of the day
💡 Did you know, you can add REVIEW.md to your repo root and Qodo follows it on every PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread tools/provider-smoke/smoke_execution_providers.py Outdated
Comment thread src/Trackdub.Benchmarks/TtsEvalRunner.cs
Comment thread src/Trackdub.Benchmarks/TtsEvalRunner.cs
Comment thread tools/provider-smoke/smoke_execution_providers.py Outdated
Comment thread src/Trackdub.Benchmarks/TtsEvalRunner.cs
Comment thread tools/provider-smoke/smoke_execution_providers.py
@qodo-code-review

Copy link
Copy Markdown
Contributor

Qodo Fixer

🍒 Ready to be cherry-picked — ✅ Merged (0) · ☑ Fixed (2)

Grey Divider

🔗 Fix PR: #392

This fix PR was closed automatically. Its branch is preserved so you can cherry pick the changes into the original PR.

Prompt for coding agent

This is an automated fix prepared on a separate branch (#392). It is NOT applied to this PR.
To use it: review Fix PR #392 (https://github.com/trackdubllc/Trackdub/pull/392), evaluate each change critically against your local context, and cherry-pick the changes that are correct into this branch. Do not accept them blindly.
Process — 2 fixed
  • ☑ Fixed: CPU-only runs appear to use an accelerator
  • ☑ Fixed: Some registered TTS engines always fail

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 7 files

You’re at about 94% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread tools/provider-smoke/README.md Outdated
Comment thread tools/provider-smoke/smoke_execution_providers.py Outdated
Comment thread src/Trackdub.Benchmarks/TtsEvalRunner.cs
Comment thread src/Trackdub.Benchmarks/TtsEvalRunner.cs
Comment thread docs/specs/design-piper-cpu-tts-tier.md
Comment thread tools/provider-smoke/smoke_execution_providers.py
Comment thread tools/provider-smoke/smoke_execution_providers.py
Comment thread tools/provider-smoke/smoke_execution_providers.py Outdated
Comment thread src/Trackdub.Benchmarks/TtsEvalRunner.cs
Comment thread src/Trackdub.Benchmarks/TtsEvalRunner.cs

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Provider status detection is inaccurate, valid inputs are mishandled, and unrelated unverified changes from separate PRs are included.

Review effort: Balanced
Findings: 1 High severity · 4 Medium severity · 5 Low severity

Open (10)
What changed in this PR

Adds a lightweight Python ONNX Runtime provider smoke harness, but also includes unrelated TTS benchmarking and Piper design work from separate PRs.

Changes:

  • Adds provider loading, inference, fallback-status, and timing checks.
  • Adds the tts-bench command and tests.
  • Adds the Piper CPU TTS design specification.
File Description
tools/​provider-smoke/​smoke_execution_providers.py Implements provider smoke runs and reporting.
tools/​provider-smoke/​README.md Documents harness usage and provider prerequisites.
src/​Trackdub.Benchmarks/​TtsEvalRunner.cs Implements TTS benchmark jobs.
src/​Trackdub.Benchmarks/​Program.cs Registers tts-bench.
src/​Trackdub.Benchmarks/​BenchmarkConsole.cs Adds command usage text.
tests/​Trackdub.Benchmarks.Tests/​TtsEvalRunnerTests.cs Tests TTS benchmark behavior.
docs/​specs/​design-piper-cpu-tts-tier.md Proposes a benchmark-first Piper fallback tier.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread tools/provider-smoke/smoke_execution_providers.py Outdated
Comment thread src/Trackdub.Benchmarks/TtsEvalRunner.cs
Comment thread src/Trackdub.Benchmarks/TtsEvalRunner.cs
Comment thread tools/provider-smoke/smoke_execution_providers.py Outdated
Comment thread tools/provider-smoke/smoke_execution_providers.py
Comment thread docs/specs/design-piper-cpu-tts-tier.md
Comment thread src/Trackdub.Benchmarks/BenchmarkConsole.cs
Comment thread src/Trackdub.Benchmarks/Program.cs
Comment thread tools/provider-smoke/README.md Outdated
Comment thread tools/provider-smoke/smoke_execution_providers.py Outdated
Comment thread tools/provider-smoke/smoke_execution_providers.py Fixed

@opencode-agent opencode-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review: 8 inline finding(s); the summary follows as a comment.

Comment thread tools/provider-smoke/smoke_execution_providers.py Outdated
Comment thread tools/provider-smoke/smoke_execution_providers.py Outdated
Comment thread tools/provider-smoke/smoke_execution_providers.py Outdated
Comment thread src/Trackdub.Benchmarks/TtsEvalRunner.cs
Comment thread src/Trackdub.Benchmarks/TtsEvalRunner.cs
Comment thread src/Trackdub.Benchmarks/BenchmarkConsole.cs
Comment thread src/Trackdub.Benchmarks/TtsEvalRunner.cs
Comment thread tools/provider-smoke/smoke_execution_providers.py Outdated
@opencode-agent

opencode-agent Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

Review submitted: pullrequestreview-5432028722

Independent read-only review of the e3d1f0a snapshot. I validated each finding against the actual codebase (engine overloads, ExecutionProviderTokens, the manifest aliases, DI registration) rather than relying on the prior bot threads. 8 inline findings posted; no out-of-diff findings.

Overall: The provider-smoke harness's core status contract is unsound — its headline "never fabricated" guarantee doesn't hold — and the stacked tts-bench feature bypasses the routed engine, breaking documented --model aliases and any non-Kokoro TTS family. Both should be addressed before this ships as evidence-gathering tooling.

# Severity Location Title
1 critical tools/provider-smoke/smoke_execution_providers.py:180-183 ok/fallback based on get_providers(), not node placement
2 important tools/provider-smoke/smoke_execution_providers.py:105-107 build_feed forces float32, breaks integer/bool inputs
3 important tools/provider-smoke/smoke_execution_providers.py:155 Comma used for both provider and option splitting
4 important src/Trackdub.Benchmarks/TtsEvalRunner.cs:220-228 tts-bench selects adapter by EngineFamily, calls unplanned overload
5 important src/Trackdub.Benchmarks/TtsEvalRunner.cs:320-321 Per-job repeat_runs: 0 fabricates Ok with zero timing
6 suggestion src/Trackdub.Benchmarks/BenchmarkConsole.cs:23 Help lists Windows ML device-policy keys as --provider tokens
7 suggestion src/Trackdub.Benchmarks/TtsEvalRunner.cs:113-122 --provider auto rejected though advertised
8 suggestion tools/provider-smoke/smoke_execution_providers.py:27 Unused ctypes, dead DEFAULT_DYNAMIC_DIM, docstring advertises unsupported JSON feed

Out of diff: None.

github run

@tonythethompson

Copy link
Copy Markdown
Contributor Author

/oc fix

Co-authored-by: tonythethompson <tonythethompson@users.noreply.github.com>
@opencode-agent

opencode-agent Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

All verification passed. Here's the summary for the PR.


Autopilot pass — provider-smoke harness

Merge conflicts: none (PR is MERGEABLE). CI: no failing checks on e3d1f0a (CodeQL, cubic, review/opencode-review green; the action_required run is an OpenCode comment-triggered workflow awaiting human approval, not a build/test failure).

Scope: this branch stacks the piper spec (#386) and tts-bench (#388) PRs underneath; this PR's own change is the provider-smoke harness. All 56 review threads triaged (35 fixed, 21 dismissed), each replied to and resolved.

Fixed (provider-smoke, 35 threads)

  • Status contract was unsound (ok/fallback from sess.get_providers()): smoke() now runs once with the ONNX Runtime profiler enabled, counts kernel events per EP, and reports ok only when the requested EP executed ≥1 node; 0 nodes → fallback with placement detail. Per-EP counts recorded under node_counts. Verified live against ORT 1.30.
  • Feed dtype coercion: explicit .npz feeds preserve stored dtypes (int64/bool ok); generated feeds map each input's declared ONNX type (TYPE_TO_NUMPY), raising "provide an explicit --feed npz" for unmapped types.
  • Missing EP recorded as fail: now absent (with wheel hint) when not in ort.get_available_providers(), and ORT library-load failures (e.g. missing ROCm libmigraphx) classify as absent — README migraphx evidence relabeled accordingly.
  • Comma delimiter conflict: provider options now use ; (openvino:device_type=CPU;performance_hint=THROUGHPUT); README/docstring updated.
  • QNN discovery: _qnn_package_dir() shared by qnn_provider_library()/qnn_backend_path(), returning None when onnxruntime_qnn isn't installed (fixed the unguarded __import__ crash).
  • Repeat/warmup validation: --repeat-runs < 1 / --warmup-runs < 0 rejected via parser.error; bench_run() guards repeats < 1.
  • Cleanup: removed unused ctypes, dead DEFAULT_DYNAMIC_DIM, docstring JSON-feed claim.
  • Verified: py_compile clean; live run → cpu ok with node_counts, openvino/qnn → absent on stock wheel, explicit int64 npz feed runs.

Not fixed (dismissed as out of scope, 21 threads)

Blocked on a human: none beyond the action_required approval on the comment-triggered OpenCode run.

github run

@tonythethompson
tonythethompson merged commit 5b04a96 into main Oct 6, 2026
19 of 22 checks passed
@tonythethompson
tonythethompson deleted the vibe/provider-smoke-harness-dfc30d branch October 6, 2026 17:45
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.

3 participants