feat: tts review skills

.agents/skills/ten-tts-review/SKILL.md

@@ -0,0 +1,120 @@
+---
+name: ten-tts-review
+description: Review implementation changes inside TEN Framework TTS extension directories for actionable correctness defects. Use when a pull request or diff changes Python/source code, extension-local configuration or manifests, dependencies, or tests under `ai_agents/agents/ten_packages/extension/` for a TTS extension. Focus on request lifecycle, exact `tts_audio_end` duration and event-interval accounting, streaming and flush races, PCM format, vendor errors, metrics, secrets, and regression coverage. Do not use for `ten_ai_base`, `tts_guarder`, graphs, examples, audio routing outside the extension, other extension types, or documentation-only changes.
+---
+
+# TEN TTS review
+
+Find defects that can affect users or make the TTS contract unreliable. Prefer a
+small number of high-confidence findings over a broad checklist report.
+
+## Review workflow
+
+1. Read the repository instructions before reviewing. At minimum, load
+   `AGENTS.md`, `docs/ai/L0_repo_card.md`, and all files in `docs/ai/L1/`.
+2. Establish the exact diff and its base. For a pull request, inspect the PR
+   metadata and full diff. For a local review, use the merge base with the target
+   branch. Do not infer a defect from a truncated patch.
+3. Build the in-scope file list first. Review only changed files under
+   `ai_agents/agents/ten_packages/extension/<tts-extension>/`, where the extension
+   name or its imports/manifest identify it as TTS. Source code, `addon.py`,
+   `config.py`, `manifest.json`, `property.json`, dependency files, and
+   extension-local tests are in scope. README-only changes do not trigger this
+   skill. Ignore every changed path outside that extension directory, including
+   `ten_ai_base`, `integration_tests/tts_guarder`, examples, and graphs.
+4. Determine whether each in-scope TTS extension is new by checking whether its
+   extension directory exists at the pull request merge base. Do not infer this
+   from the number of added files: a replacement, rename, or large update to an
+   existing directory is not a new extension.
+5. Classify each in-scope extension from its actual inheritance and imports, not
+   its directory name:
+   - `AsyncTTS2HttpExtension` -> HTTP implementation
+   - direct `AsyncTTS2BaseExtension` -> WebSocket/custom implementation
+6. Load [references/common.md](references/common.md) and
+   [references/tests.md](references/tests.md) for every review. Then load exactly
+   the applicable transport guide:
+   - HTTP: [references/http.md](references/http.md)
+   - WebSocket/custom: [references/websocket.md](references/websocket.md)
+   If a pull request changes multiple TTS extensions, classify and review each
+   independently; load both transport guides only when both shapes are present.
+   For every new TTS extension, also load and apply
+   [references/guarder.md](references/guarder.md).
+7. Read the changed functions in full. You may inspect the installed base-class
+   methods they call to understand the contract, but do not review `ten_ai_base`
+   itself or ask the pull request to change it. Compare with one recent extension
+   using the same transport shape; copied legacy patterns are not proof.
+8. Follow each stateful path through success, empty input, vendor error,
+   cancellation, and shutdown. For HTTP extensions, distinguish inherited base
+   behavior from extension-owned behavior before raising a finding.
+9. Audit every changed call to `send_tts_audio_end()`. Independently verify
+   `request_total_audio_duration_ms` and `request_event_interval_ms`; these fields
+   are mandatory correctness checks, including partial, interrupted, error, and
+   empty-audio paths. When an HTTP extension inherits this logic unchanged, do
+   not demand a duplicate implementation in the extension.
+10. Treat extension-local tests as a merge requirement. Every in-scope TTS
+   extension must have its own tests and satisfy the mandatory matrix in
+   `references/tests.md`; shared guarder or integration tests do not substitute
+   for them. Report a missing suite or mandatory category even when the
+   production defect has not yet been demonstrated.
+11. For each new TTS extension, inspect pull request comments and enforce the
+   guarder evidence gate. The full guarder suite must pass against the current
+   pull request HEAD, and a comment must include both a result screenshot and a
+   downloadable non-empty PCM output file. Guarder evidence does not replace
+   extension-local tests.
+12. Report implementation issues only when introduced or made materially worse
+   by in-scope diff hunks. The testing baselines are explicit exceptions: report
+   extension-local test gaps for every touched TTS extension and guarder-evidence
+   gaps for every new TTS extension.
+   Verify each finding against surrounding code; do not flag style, preferences,
+   or purely hypothetical risks.
+
+## Evidence standard
+
+A finding should form a complete causal chain:
+
+`changed code -> reachable runtime condition -> violated invariant -> user-visible impact`
+
+Before reporting, answer all of these:
+
+- What input, event ordering, or vendor response triggers it?
+- Why do guards, the base class, or cleanup code not prevent it?
+- Which request, audio, error, metric, or secret is affected?
+- Is the faulty behavior owned by the changed extension rather than an unchanged
+  installed base class?
+- Can the location be narrowed to changed lines?
+
+If any answer is missing, investigate further or omit the finding.
+
+## Severity
+
+- **P0**: widespread outage, credential exposure, or destructive behavior.
+- **P1**: common requests break, cross-request audio leaks, flush is violated, or
+  the extension cannot recover without restart. A new or changed TTS extension
+  with no extension-local test suite is also P1. A new TTS extension without
+  current, passing guarder evidence and both required comment attachments is P1.
+- **P2**: a realistic edge case produces wrong audio/events/metrics or resource
+  leakage with meaningful impact, or the mandatory local-test matrix is
+  materially incomplete.
+- **P3**: limited but concrete correctness or maintainability defect worth fixing.
+
+Do not inflate severity because code is concurrent or vendor-facing.
+
+## Output
+
+List findings first, ordered by severity. Use this form:
+
+```text
+[P1] Imperative, specific title
+path/to/file.py:123
+Explain the triggering sequence, the incorrect behavior, and its impact in one
+compact paragraph. Mention the relevant contract or test when useful, and give a
+bounded fix direction without writing a patch.
+```
+
+Keep line ranges tight and inside the diff whenever possible. Do not combine
+independent bugs in one finding. Do not add praise, a walkthrough of the patch, or
+generic testing advice. If there are no actionable findings, say:
+
+```text
+No actionable TTS-specific findings.
+```

.agents/skills/ten-tts-review/evals/evals.json

@@ -0,0 +1,67 @@
+{
+  "skill_name": "ten-tts-review",
+  "evals": [
+    {
+      "id": 1,
+      "prompt": "Review commit 3f9b4c6c4 as a pull-request diff against its first parent. Apply the TTS review only to changed files inside the fish_audio_tts_python extension directory.",
+      "expected_output": "A concise, line-specific review that traces fish_audio TTS flush and terminal-event behavior and avoids generic checklist comments.",
+      "files": [],
+      "expectations": [
+        "Every finding identifies a reachable event ordering and user-visible consequence",
+        "The review classifies fish_audio_tts_python as WebSocket/custom and applies the direct-base transport checks",
+        "The review checks exactly-once audio_end and post-flush emission behavior",
+        "The review does not ask cancel_tts to call finish_request when the base class owns cleanup",
+        "The review produces no findings for ten_ai_base, tts_guarder, examples, or graph files",
+        "The output uses P0-P3 priorities and tight changed-line locations"
+      ]
+    },
+    {
+      "id": 2,
+      "prompt": "Review commit dc488ff14 as if it were a new TTS extension PR. Review only files under ai_agents/agents/ten_packages/extension/mistral_tts_python and ignore every other changed path.",
+      "expected_output": "A high-signal review of the initial Mistral/Voxtral TTS implementation, especially emitted audio representation, sample-rate truth, streaming, secrets, and tests.",
+      "files": [],
+      "expectations": [
+        "The review verifies that vendor audio bytes match TEN pcm_frame metadata",
+        "The review classifies mistral_tts_python as HTTP and applies the HTTP client stream contract",
+        "The review does not require lifecycle methods already inherited unchanged from AsyncTTS2HttpExtension",
+        "The review considers sample conversion across streamed chunk boundaries",
+        "The review checks that request-only fields and credentials are not forwarded or logged",
+        "The review treats extension-local tests as required and checks the full common matrix plus HTTP-specific cases",
+        "The review recognizes the extension directory as new and requires current passing guarder evidence with a screenshot and downloadable PCM output in a pull request comment",
+        "The tts_guarder runner and documentation changes in the commit receive no findings",
+        "Generic praise and non-actionable style feedback are absent"
+      ]
+    },
+    {
+      "id": 3,
+      "prompt": "Review commit 26cca0807 against its first parent. Limit this TTS review to ai_agents/agents/ten_packages/extension/gradium_tts_python, even though the commit also changes examples and generated manifests.",
+      "expected_output": "A focused review of latency, TTFB/event ordering, sample-rate handling, configuration, failure completion, and regression coverage in the initial Gradium TTS change.",
+      "files": [],
+      "expectations": [
+        "The review follows success, vendor error, iterator end, and cancellation paths",
+        "The review classifies gradium_tts_python as WebSocket/custom rather than applying the HTTP-base checklist",
+        "The review detects whole-response buffering or duplicate TTFB when present",
+        "The review separately validates audio duration from emitted PCM bytes and event interval from audio_start to audio_end",
+        "The review treats extension-local tests as required and checks the full common matrix plus WebSocket-specific lifecycle cases",
+        "The review recognizes the extension directory as new and requires current passing guarder evidence with a screenshot and downloadable PCM output in a pull request comment",
+        "Each reported issue explains why base-class guards do not prevent it",
+        "Example graph and manifest-lock changes receive no findings",
+        "The output says no actionable findings instead of inventing issues when evidence is insufficient"
+      ]
+    },
+    {
+      "id": 4,
+      "prompt": "Review commit 85c777953 as a TTS extension change, limited to ai_agents/agents/ten_packages/extension/cosy_tts_python. Pay particular attention to every tts_audio_end payload.",
+      "expected_output": "A line-specific review that detects audio-end interval measurement from the vendor request timestamp instead of the audio-start timestamp and verifies the PCM duration formula independently.",
+      "files": [],
+      "expectations": [
+        "The review defines request_event_interval_ms as elapsed time from tts_audio_start to tts_audio_end",
+        "The review applies the WebSocket/custom lifecycle and concurrency guide to the direct AsyncTTS2BaseExtension subclass",
+        "The review explains that measuring from request_start_ts incorrectly includes TTFB",
+        "The review checks request_total_audio_duration_ms using emitted PCM bytes, sample rate, sample width, and channels",
+        "The review checks success, partial error, interruption, and no-audio terminal paths",
+        "Findings remain limited to the cosy_tts_python extension directory"
+      ]
+    }
+  ]
+}

.agents/skills/ten-tts-review/references/common.md

@@ -0,0 +1,152 @@
+# Common TEN TTS extension review guide
+
+Load this guide for every in-scope TTS extension review. Apply it only to changed
+files inside the extension directory.
+
+## Scope and source of truth
+
+An extension is in scope when its manifest imports the TTS interface or its
+implementation inherits a TTS base class. Review implementation code, vendor
+clients, addon/configuration code, extension manifest/property files, dependency
+metadata, and extension-local tests.
+
+Do not produce findings for `ten_ai_base`, `integration_tests/tts_guarder`,
+examples, graphs, or any other directory. The installed base implementation may
+be read only to understand which behavior the extension inherits.
+
+Use these sources in descending order of authority:
+
+1. the changed extension and its local tests;
+2. the public contract of the installed base class it consumes;
+3. a recent extension with the same transport shape;
+4. repository documentation and older implementations.
+
+Copied code is comparison evidence, not proof that a pattern is correct.
+
+## Request lifecycle and ordering
+
+Trace every request ID separately through success, empty input, vendor error,
+cancellation, and shutdown.
+
+Verify:
+
+- Non-empty output sends `tts_audio_start` before the first PCM frame and exactly
+  one `tts_audio_end` after the last frame.
+- Appended text with the same request ID stays one request. A non-final segment
+  does not complete it, and a final segment does not leave it stuck.
+- Empty or whitespace final input does not hang or emit bogus audio.
+- Explicit vendor end, iterator exhaustion, error, timeout, and exception paths
+  converge on one terminal path without duplicate end events or cleanup.
+- Request metadata and frames remain associated with the correct request when
+  request IDs are interleaved.
+- Completion releases subsequent work and cleans up request state, recorders,
+  counters, timestamps, and pending messages owned by the extension.
+
+Do not require an HTTP extension to reproduce lifecycle work that it inherits
+unchanged from `AsyncTTS2HttpExtension`.
+
+## `tts_audio_end` accounting — mandatory
+
+Audit every changed extension-owned path that sends `tts_audio_end`. The two
+numeric fields have different meanings and must be calculated independently.
+
+### `request_total_audio_duration_ms`
+
+This is the media duration represented by PCM frames actually emitted for the
+request:
+
+```text
+duration_ms = emitted_pcm_bytes * 1000
+              / (sample_rate_hz * bytes_per_sample * channels)
+```
+
+Verify:
+
+- Count bytes after decoding, decompression, resampling, format conversion, and
+  frame alignment. Do not count headers, compressed bytes, base64 text,
+  conversion input, or a partial sample that was never emitted.
+- Use the rate, width, and channels attached to that request's emitted frames.
+- Accumulate the whole request ID, including appended text, and reset only at the
+  request boundary.
+- Sum bytes first and convert once; per-chunk rounding accumulates error.
+- Success reports all emitted PCM. Error/interruption reports only PCM already
+  emitted. No emitted audio reports `0`.
+- Duplicate terminal handlers cannot reset or reuse counters before the winning
+  path constructs its payload.
+
+### `request_event_interval_ms`
+
+This is elapsed real time from emitting `tts_audio_start` to emitting the matching
+`tts_audio_end`. It is neither audio duration nor total vendor request latency.
+
+Verify:
+
+- Capture the start exactly once when the first `tts_audio_start` is sent. Starting
+  at connect, vendor request, or text-send time incorrectly includes TTFB.
+- Capture the matching end when `tts_audio_end` is sent. Appended segments do not
+  replace the original start.
+- Prefer a monotonic clock; wall-clock adjustment can corrupt elapsed time.
+- Success, error-after-audio, and interrupted-after-audio use the same start/end
+  boundary. If no audio-start event was sent, report `0`.
+- Keep timestamps per request or prove that callbacks cannot pair different
+  requests.
+- Do not substitute PCM duration: synthesis may run faster or slower than real
+  time.
+
+A wrong formula, origin, association, or reset boundary is a correctness defect,
+not a metrics preference.
+
+## Audio correctness and latency
+
+Verify:
+
+- `pcm_frame` bytes are raw signed PCM matching the declared sample rate, width,
+  channels, and endianness—not WAV/MP3 headers or vendor-specific floats.
+- Resampling and conversion preserve samples split across chunk boundaries.
+- Float conversion maps non-finite values safely, clamps before scaling, and does
+  not overflow or wrap.
+- Frame lengths align to `sample_width * channels`; final leftover handling does
+  not invent, drop silently, or count an incomplete sample.
+- Useful chunks are forwarded promptly instead of buffering the whole response.
+- A configurable sample rate is proven by extension-local tests; a fixed rate is
+  acceptable when it matches the vendor.
+
+## Configuration, security, errors, and metrics
+
+Verify:
+
+- Manifest name, addon decorator, dependencies, and imported interface agree
+  within the extension.
+- Required configuration is validated before use. Internal fields such as API
+  keys, dump controls, base URL, and framework-only audio settings are not
+  forwarded as vendor payload parameters unless the vendor requires them.
+- Keys, authorization headers, signed URLs, tokens, sensitive config, and user
+  text are not exposed in logs, exceptions, metadata, or fixtures.
+- Provider errors include useful vendor information and use fatal/non-fatal
+  severity consistent with recoverability.
+- Error paths terminate the current request without poisoning later requests.
+- TTFB is measured from the actual vendor request to the first non-empty audio and
+  emitted once per request. It must not be reused for audio-end event interval.
+- Usage, byte, chunk, character, and duration counters are request-scoped.
+- Dump writers write emitted PCM, are isolated per request, and flush on every
+  terminal path without path traversal through request IDs.
+
+## Extension-local tests
+
+Apply the complete merge-gating matrix in [tests.md](tests.md). The suite must
+exercise the extension-owned config, client/stream adapter, conversion, event,
+and cleanup paths with deterministic fakes; tests of only a base class, shared
+guarder, or external integration graph do not satisfy the requirement.
+
+## Common false positives
+
+Do not report:
+
+- inherited base behavior as a defect in an extension that does not override it;
+- lack of subtitle alignment when the vendor exposes no timestamps;
+- a fixed rate that matches a fixed-rate vendor;
+- absence of `tts_audio_start` for a request that emits no frames;
+- old defects outside the diff unless the change makes them reachable or worse;
+- speculative races without a concrete ordering and observable impact;
+- formatting or naming preferences already enforced by CI;
+- findings against out-of-scope directories.