feat(tts): report per-request metrics

[YiminW]

Summary

• report each Rime WebSocket text send through the common TTS request metrics API
• bump rime_tts from 0.4.10 to 0.4.11

Dependency

• Requires feat: add TTS request reporting metrics ten_ai_base#118

Testing

• Rime tests/test_params.py (1 passed)
• modified Python files compile successfully
• updated manifest JSON and pyproject TOML parse successfully

OpenAI TTS request reporting is inherited entirely from ten_ai_base and requires no package change here. Bytedance duplex is owned by the conversational_agent repository.

[wangyimin-agora]

send_tts_request_final_marker() 是一个内部“结束信号”，用于通知 report_controller：
当前 request_id 后续不会再产生 TTS vendor request，可以立即上报缓存的数据。
不会生成一条实际的 requests[] 数据。

[github-actions]

Review: feat(tts): report per-request metrics

Small, focused diff, and the direction is right — routing per-request reporting through one base API instead of per-extension code. A few things worth resolving before merge.

Note on verification: send_tts_request_metrics comes from ten_ai_base (TEN-framework/ten_ai_base#118), which is not vendored in this repo and is not installed in my environment, so I could not check the signature, the type of request_final, or the base-class behavior. Comments below are based on the call sites in this repo.

1. Version pinning — the new API isn't pinned anywhere

bytedance_tts_duplex/manifest.json and rime_tts/manifest.json still declare ten_ai_base "version": "0.7", unchanged. If send_tts_request_metrics ships in a newer ten_ai_base, both extensions can resolve a base that lacks the method and fail at runtime with AttributeError inside request_tts — i.e. mid-request, in the audio path. The dependency constraint should be bumped to the version that actually contains the method.

Related inconsistency: openai_tts2_python was bumped 0.6.9 → 0.6.10, but its only change in this PR is reordering two imports — no behavior change in this repo. Meanwhile bytedance_tts_duplex (0.4.2) and rime_tts (0.4.10), which do change behavior, got no bump. That looks inverted.

2. request_final=True can never be emitted for empty final chunks

In bytedance_tts_duplex/extension.py, the new call sits inside if t.text.strip() != "":. In rime_tts/extension.py it sits inside if t.text:. A flush-style final message carrying text_input_end=True with empty or whitespace-only text therefore sends no request metrics at all — so request_final is never observed as True for that request.

Bytedance has a second gap: the empty-first-message early-return path (around L418-L440) returns after send_tts_audio_end / send_usage_metrics / finish_request without ever calling send_tts_request_metrics. A fully-empty request emits no request metrics.

If any consumer uses request_final to close out a request record, these paths will leak open records. Two options: emit the metrics whenever text_input_end is set (with output_characters=0), or document that request_final is best-effort and consumers must key off tts_audio_end instead.

3. Divergent guards between the two extensions

Bytedance: t.text.strip() != "" — whitespace-only text excluded.
Rime: if t.text: — whitespace-only text included. (metrics_add_output_characters(len(t.text)) on the line above is unconditional in both cases.)

Same logical event, two different definitions of "countable text". Since this feeds metrics presumably aggregated across vendors, pick one predicate and use it in both. t.text.strip() matches the existing bytedance convention and the metrics_add_output_characters intent more closely.

4. Metrics are reported before the send succeeds

Both call sites do await self.send_tts_request_metrics(...) and then await self.client.send_text(...). If the WebSocket send raises (closed connection, vendor error), a request has been reported that never reached the vendor. For anything billing-adjacent, reporting after a successful send is safer. If pre-send ordering is deliberate (e.g. so the timestamp reflects intent rather than completion), a one-line comment saying so would help.

5. Timestamp source is inconsistent with surrounding code

int(time.time() * 1000) is new; both files otherwise use datetime.now() and, at bytedance_tts_duplex/extension.py:252, int(datetime.now().timestamp() * 1000). Functionally equivalent, but it adds a second time import and a second idiom to files that already have one. Reusing datetime.now().timestamp() avoids the new import time entirely.

6. Test coverage

Both extensions have a tests/test_metrics.py, and the PR description only reports test_params.py runs — which don't exercise this path. Nothing currently asserts that the new message is emitted, with the right request_id, or with request_final=True at end of input. The existing metrics tests already capture emitted messages (rime_tts/tests/test_metrics.py parses the metrics data message), so extending that pattern should be cheap. Given that issue #2 is exactly the kind of bug a test would catch, this seems worth adding.

Minor

• int(time.time() * 1000) yields a slightly different timestamp on each call within one request. Presumably intended (per-send timestamps) — flagging only in case the field is meant to be the request start time, since self.request_start_ts is already tracked in both extensions.
• Placement relative to metrics_add_output_characters is consistent and readable; no style complaints.

Nothing here breaks the happy path, but #1 (unpinned dependency) and #2 (missing request_final) are the two I'd want addressed before this lands.

[github-actions]

Review: feat(tts): report per-request metrics

Small, focused diff. The Rime side is the only real code change — the OpenAI side is an import reorder plus a version bump, since the reporting itself lands in ten_ai_base. A few things worth resolving before merge.

1. The final chunk of a request often carries no metric at all

if t.text:
    await self.send_tts_request_metrics(..., request_final=t.text_input_end)

The if t.text: guard means an empty final input never reports. That case is real and explicitly handled a few lines below:

if self.request_start_ts and t.text.strip() == "" and not self.sent_tts:
    # Empty text with text_input_end - finish immediately

So for a request whose closing message is text="" / text_input_end=True — a common flush pattern — no metric with request_final=True is ever emitted. Any downstream consumer that keys off request_final to close out a request will never see the terminal event for those requests. Suggest emitting whenever the input is final:

if t.text or t.text_input_end:
    await self.send_tts_request_metrics(...)

Related inconsistency: the line immediately above (metrics_add_output_characters(len(t.text))) has no guard, so character metrics and request metrics now disagree on which inputs count. And if t.text: treats " " as non-empty while the code below uses .strip() — worth picking one notion of empty.

2. Interrupted requests never get a terminal metric

cancel_tts() sets current_request_finished = True and emits TTSAudioEndReason.INTERRUPTED, but no request metric with request_final=True. Combined with #1, per-request aggregation on the consumer side can hold open state indefinitely for interrupted turns. If the base class doesn't already emit a final report on cancel, this path needs one too.

3. request_time_ms semantics — please confirm against ten_ai_base#118

int(time.time() * 1000) is a Unix epoch timestamp (~1.75e12). Every neighbouring metric in this extension is an elapsed duration (ttfb, _calculate_audio_duration_ms), and the name request_time_ms reads like a duration too. If the base API expects "ms since request start", this is off by twelve orders of magnitude and the bug stays invisible until someone plots it. If it genuinely wants wall-clock, a one-line comment saying so would help.

Minor: the extension already tracks time via datetime (self.request_start_ts = datetime.now()). int(datetime.now().timestamp() * 1000) avoids the new import time and keeps one clock source.

4. Telemetry failure is reported to the user as a TTS error

The call sits inside the big try in request_tts, so if send_tts_request_metrics raises it is caught by the generic handler and turned into a ModuleError(NON_FATAL_ERROR) — and, when text_input_end is set, into _handle_tts_audio_end(reason=ERROR), ending an otherwise-healthy synthesis. Not hypothetical while the base-package dependency is unreleased: on an older ten_ai_base the attribute doesn't exist and every request fails with AttributeError. Either guard the call or handle metrics failures separately from synthesis failures.

5. Dependency pin doesn't express the new requirement

Both manifest.json files still declare ten_ai_base "version": "0.7", and neither requirements.txt pins it, yet the code now hard-depends on an API added in ten_ai_base#118. Nothing prevents installing a 0.7.x without send_tts_request_metrics (see #4 for the failure mode). spatius_avatar_python pins "0.7.40", so a more specific minimum appears to be supported — consider bumping both extensions to the first base version containing the API.

6. Ordering: metric is emitted before the send is attempted

The report happens before await self.client.send_text(t). send_text only enqueues, so the metric already describes an intent rather than a delivered WS frame (the PR title says "each WebSocket text send"). If send_text throws, the metric was already counted. Reporting after the enqueue would at least align with the local success path.

7. Test coverage

No test exercises the new call. The listed runs (test_params.py) don't touch request_tts metrics. rime_tts/tests/test_metrics.py already has the harness for asserting on metrics data messages — two cases would cover the risky parts of this diff: (a) a metric is emitted per text input, and (b) request_final=True arrives exactly once, including when the closing input is empty. Given #1 and #2 are both "missing event" bugs, tests are the only thing that will catch a regression here.

Nits

• The OpenAI extension.py change is pure import reordering with no behavioural effect; fine, but it makes the version bump look like a code change when the actual behaviour comes from the base package. Worth noting in the commit body.
• rime_tts/extension.py still has a Chinese comment (# 创建新的 PCMWriter) near the edited block — not from this PR, but cheap to clean up while you're here.

[github-actions]

Review: feat(tts): report per-request metrics

Small, focused PR. The Rime hook is placed sensibly next to the existing character-metric call, and the version bumps are consistent across manifest.json and pyproject.toml for both extensions. A few things worth resolving before merge — the first two are the substantive ones.

1. Requests closed with an empty final chunk never emit request_final=True

if t.text:
    await self.send_tts_request_metrics(
        ...
        request_final=t.text_input_end,
    )

The if t.text: guard skips reporting for empty text — but text_input_end=True frequently arrives on a chunk with empty text (that's exactly the case the block ~10 lines below is written for: "Empty text with text_input_end - finish immediately"). In that flow, a consumer aggregating per-request metrics and waiting on request_final to close the record never receives a terminal marker, so the request is left open indefinitely.

Suggest decoupling the two conditions: always report when t.text_input_end is set, and use the text guard only to avoid zero-character noise on intermediate chunks:

if t.text or t.text_input_end:
    await self.send_tts_request_metrics(...)

Related: if t.text is truthy for whitespace-only text (" "), while the surrounding code uses t.text.strip() == "" to decide emptiness. Worth aligning on one definition.

2. A metrics failure now drops the audio chunk

The await self.send_tts_request_metrics(...) sits before await self.client.send_text(t), inside the same try. If the metrics call raises for any reason, send_text is never reached and the exception is converted into a NON_FATAL_ERROR / audio-end — the user loses that speech segment because telemetry failed. Metrics reporting should be best-effort and never on the critical path. Two options:

• Move the call after await self.client.send_text(t), which also makes the report reflect a send that actually happened rather than one that may still throw; or
• Wrap it so it can't propagate:

try:
    await self.send_tts_request_metrics(...)
except Exception as e:  # metrics must not block synthesis
    self.ten_env.log_warn(f"failed to report tts request metrics: {e}")

3. request_time_ms semantics

int(time.time() * 1000) is a wall-clock epoch timestamp, not an elapsed duration. If the field is meant as "when the request was sent," this is correct, but the _ms suffix reads like a duration — could you confirm against the ten_ai_base signature? Two related notes:

• time.time() is not monotonic; it can jump backwards on NTP correction. If this value is ever used to compute intervals, time.monotonic() is safer for the delta, with epoch time only for the label.
• The extension already tracks self.request_start_ts = datetime.now() for the request. If request_time_ms is intended to be the request start, reusing that field would keep per-chunk reports consistent instead of re-reading the clock on every chunk of the same request.

4. Possible double-counting of characters

self.metrics_add_output_characters(len(t.text)) is retained immediately above, and the new call passes output_characters=len(t.text). If both feed the same downstream aggregation, characters get counted twice. Intentional (different sinks), or should the older call go away once the new API lands?

5. Dependency floor is not expressed

send_tts_request_metrics does not exist anywhere in this repo — it arrives from ten_ai_base via TEN-framework/ten_ai_base#118 — yet manifest.json still pins "ten_ai_base": "0.7", unchanged. Nothing in this PR prevents rime_tts 0.4.11 from resolving against a ten_ai_base that predates #118, which would fail at runtime with an AttributeError on the first TTS chunk rather than at install time. If the pin scheme supports a patch floor, raising it here would make the coupling explicit; otherwise it'd be good to confirm merge ordering with #118.

6. OpenAI change is a no-op in this diff

The summary says "let OpenAI TTS inherit request reporting from the HTTP base implementation," but the only change to openai_tts2_python/extension.py is reordering two imports — the behavior comes entirely from the base class. That's fine, though it means the 0.6.9 → 0.6.10 bump exists purely to re-pull ten_ai_base. Worth stating that explicitly in the description so reviewers don't hunt for a functional change that isn't there. The import reorder itself is cosmetic; if it isn't required by a linter, it's unrelated churn.

7. Test coverage

No test covers the new behavior. rime_tts/tests/test_metrics.py already has the harness for asserting on emitted metrics data (it does this for ttfb), so a test that sends two chunks and asserts one request-metrics message per non-empty chunk — plus exactly one with request_final=True — would be cheap to add and would directly pin down issue #1. test_params.py passing (1 test) does not exercise this path.

Minor / pre-existing

Not introduced here, but adjacent to the code you touched: self.sent_tts = True is set unconditionally just above, so the later ... and not self.sent_tts guard on the empty-text finish branch can never be true — that branch appears to be dead. Worth a separate look, since it is the same empty-final-chunk scenario as #1.

Checklist

Per docs/ai/L1/04_conventions.md, please confirm task format && task check && task lint were run in the container — task lint fails CI on any warning, and this PR adds a new import.

[github-actions]

Review: feat(tts): report per-request metrics

Focused change and the version bumps are consistent (manifest.json and pyproject.toml both land on 0.4.11). A few things worth addressing before merge, one of which I think is a real correctness gap.

1. request_final=True is lost when the final chunk has empty text

if t.text:
    await self.send_tts_request_metrics(
        ...
        request_final=t.text_input_end,
    )

The if t.text: guard suppresses the whole call for empty text — including the terminal marker. But this extension explicitly supports a final chunk with empty text; the code ~10 lines below handles exactly that case:

if self.request_start_ts and t.text.strip() == "" and not self.sent_tts:
    # Empty text with text_input_end - finish immediately

So for a request whose closing message is text="", text_input_end=True, no metrics event with request_final=True is ever emitted. Any downstream consumer that aggregates per request and keys finalization off request_final would never see the request close — stale accumulator or a request that never finalizes. Whitespace-only text (" ") is truthy and dodges this, which makes the behavior inconsistent in a way that will be annoying to debug.

Suggest gating on characters but still reporting the terminal flag:

if t.text or t.text_input_end:
    await self.send_tts_request_metrics(
        request_id=t.request_id,
        request_time_ms=...,
        output_characters=len(t.text),
        request_final=t.text_input_end,
    )

Minor related point: if t.text could ever be None, the unguarded len(t.text) on the immediately preceding line already raises, so the guard is not buying None-safety.

2. Metrics are reported before the send, so failed sends are counted

The call sits before await self.client.send_text(t). If send_text raises (ModuleVendorException, dropped socket), the characters have already been reported for a send that never reached the vendor. The except branch resets self.sent_tts = False but cannot un-report the metric. If these numbers feed usage or billing, that is systematic over-counting on every connection blip.

Moving the block to after send_text returns fixes this and also addresses the next point. If reporting attempts rather than successes is intentional, worth a comment saying so.

3. Awaiting on the hot path may skew the TTFB metric

send_tts_request_metrics is awaited before the text reaches the vendor, on every chunk. Since this extension also reports TTFB (EVENT_TTS_TTFB_METRIC, asserted in tests/test_metrics.py), any latency introduced here is added to the measured TTFB. Probably small, but it is avoidable — reporting after the send removes it entirely.

4. request_time_ms semantics and the new time import

int(time.time() * 1000) is wall-clock epoch ms. That is fine if the field is an absolute timestamp, but the name reads equally well as a duration, in which case this value is meaningless. Please confirm against the ten_ai_base definition.

If it is a timestamp, consider deriving it from the existing self.request_start_ts (datetime.now()) rather than adding import time — the file already imports datetime, and mixing both clocks for related timings invites drift. Also note wall clock is non-monotonic; NTP adjustments can produce out-of-order or negative deltas if consumers difference these values.

5. Dependency pin does not gate the new API

manifest.json still pins ten_ai_base at "version": "0.7". send_tts_request_metrics does not exist in this repo (I grepped — it comes entirely from TEN-framework/ten_ai_base#118, which is not merged yet). If that method ships in a 0.7.x patch, the existing constraint is satisfied by an older 0.7 build that lacks it, and every TTS request raises AttributeError inside request_tts. Two asks:

• Bump the constraint to the minimum ten_ai_base version that actually exposes the method.
• Confirm merge ordering — this PR should not land before the base change is published, or rime_tts breaks at runtime.

6. Test coverage

The listed testing (test_params.py, 1 passed) does not exercise this change; it passes with or without the diff. Compile checks and JSON/TOML parse checks do not cover behavior either.

The harness to test this already exists — tests/test_metrics.py::ExtensionTesterMetrics.on_data inspects the metrics data payload, and gradium_tts_python/tests/test_state_machine.py mocks metrics_add_output_characters with MagicMock as a pattern to copy. Two cases worth adding:

• request metrics emitted with the expected output_characters and request_id for a normal chunk
• request_final=True observed for a request closed by an empty-text text_input_end message (this fails today, per item 1)

Conventions

Commit style matches AGENTS.md. Since the diff only touches rime_tts, feat(rime_tts): report per-request metrics would describe the scope more accurately than the broad feat(tts): — the body already explains why OpenAI and Bytedance need no change here, which is helpful context.

Items 1 and 5 are the blocking ones for me; the rest are worth a look but not gating.

[github-actions]

Review: feat(tts): report per-request metrics

Small, focused change — the version bumps are consistent across manifest.json and pyproject.toml, and the placement inside the existing request_tts flow is reasonable. A few things worth addressing before merge.

1. ten_ai_base dependency is not bumped (blocking)

send_tts_request_metrics does not exist anywhere in this repo today — it comes from the unmerged TEN-framework/ten_ai_base#118. But manifest.json still pins ten_ai_base at version "0.7".

If #118 ships the new method in a version outside the 0.7 range that resolves here, any deployment picking up rime_tts 0.4.11 with an older ten_ai_base fails at runtime with AttributeError on the first TTS request — and because the call sits inside request_tts, the broad except Exception below swallows it into a generic non-fatal TTS error rather than an obvious dependency failure. Please confirm which ten_ai_base version carries the API, bump the constraint if it is not 0.7, and land #118 first.

2. Metrics are reported before the send can fail

await self.send_tts_request_metrics(...)
await self.client.send_text(t)

If send_text raises (ModuleVendorException on a dead WebSocket is handled explicitly a few lines below), a request has already been counted for text that never reached Rime. Moving the report after the awaited send makes the metric reflect work actually performed. If the intent is "attempted requests", that is a defensible choice — worth a short comment saying so.

3. The if t.text: guard is inconsistent with the line above it

self.metrics_add_output_characters(len(t.text))   # unguarded
if t.text:
    await self.send_tts_request_metrics(..., output_characters=len(t.text))

Two character counters that disagree on empty text. Since len("") == 0, the guard mostly just avoids a no-op event — fine, but guarding both or neither would keep the two reconcilable downstream.

Related and more important: for a streaming request, request_tts is called once per text chunk with the same request_id, so one logical request emits N metrics events with N different request_time_ms values. Does the consumer sum output_characters per request_id, or treat each event as a distinct request? If the latter, request counts inflate by chunk count.

4. Use the module already imported for time

The file uses datetime throughout (self.request_start_ts = datetime.now()); this adds a second time module for one call. int(datetime.now().timestamp() * 1000) avoids the new import and matches local convention.

Also worth deciding deliberately: the extension already records request_start_ts. If request_time_ms means when the request started, request_start_ts is the value you want — the current code reports per-chunk send time, which differs for every chunk after the first.

5. Test coverage

No test asserts the new event. tests/test_metrics.py already inspects the metrics data payload for nested ttfb, so extending it to assert a request-metrics event with the expected request_id and output_characters would be cheap — and would catch the dependency mismatch in item 1 at test time instead of at runtime.

The PR notes only test_params.py was run. Since this touches the shared request_tts path, please also run test_metrics.py, test_state_machine.py, and test_basic.py against a ten_ai_base build containing #118.

6. Rollout consistency (non-blocking)

Nine other TTS extensions call metrics_add_output_characters but none report request metrics. Until they do, dashboards built on this signal show rime only. If this is a deliberate pilot, a note in the PR body or a tracking issue for the remaining vendors would help.

Summary: items 1 and 2 are worth fixing before merge; 3-6 are improvements. No security concerns — no secrets, no new network egress, no untrusted input handling. Commit message and branch name both follow the repo conventions.

[github-actions]

Review: feat(tts): report per-request metrics

Small, focused diff, and the version bump is applied consistently to both manifest.json and pyproject.toml. Most of my feedback concerns merge ordering and the semantics of the new call rather than the mechanics of the change.

1. Blocking: the new API does not exist in this repo yet, and the failure mode is silent

send_tts_request_metrics appears nowhere in this repository tree. ten_ai_base is an external system dependency, and manifest.json still pins it at version "0.7" (unchanged by this PR). So if this merges before ten_ai_base PR 118 ships in a release that 0.7 resolves to, every call to request_tts hits AttributeError.

Two things make that worse than a normal missing-dependency error:

• The call sits inside the large try block, so the generic except Exception at the bottom of request_tts catches the AttributeError and converts it into a NON_FATAL_ERROR / tts_error. TTS stops producing audio without an obvious root cause in the logs.
• The call is placed before await self.client.send_text(t). An exception here means the text never reaches Rime at all. Telemetry becomes load-bearing for audio output.

Two recommendations, independent of each other and both worth doing:

• Move the metrics report to after await self.client.send_text(t) so a metrics failure can never block synthesis.
• Gate the merge on the base release and bump the ten_ai_base constraint in manifest.json accordingly. As written, nothing in this repo expresses the new dependency.

2. Per-send vs per-request granularity

The PR title says per-request metrics; the body says each WebSocket text send. Those are different, and the code implements the second. Rime is a streaming WS extension, so request_tts is invoked once per text chunk within a single request_id, with text_input_end marking the last one. The new call therefore fires once per chunk, every time with the same request_id, each carrying only that chunk in request_text.

Worth confirming against the base API contract: if the downstream sink keys on request_id and upserts, later chunks overwrite earlier ones and the recorded text ends up being only the final fragment rather than the full utterance. If the intent really is one report per request, emit once (first chunk, or at text_input_end with accumulated text). If per-send is intended, the title is misleading.

3. Full utterance text in the metrics pipeline

request_text=t.text puts the complete synthesis text into the metrics channel. TTS input frequently contains PII, since it is whatever the agent is about to read aloud. This repo has an explicit convention for sensitive data in logs (to_str() plus utils.encrypt() for secrets), which suggests telemetry sinks are not automatically assumed safe for arbitrary user content.

Please confirm the metrics sink has the same handling and retention expectations as logs. If it does not, consider truncating, hashing, or reporting character count only. This is a question rather than a defect claim, since the sink lives in ten_ai_base and I could not inspect it from here.

4. Time base

request_time_ms=int(time.time() * 1000) introduces a second time source. The extension already tracks self.request_start_ts = datetime.now() and uses it for TTFB. Two points:

• time.time() is not monotonic. If anything downstream computes a duration by differencing these values, an NTP correction or clock step yields a wrong or negative result. For an absolute event timestamp this is fine; for anything used as a duration anchor it is not.
• The new import time is redundant given datetime is already imported. int(datetime.now().timestamp() * 1000) keeps one time source in the file. Minor, but it also makes the relationship to request_start_ts clearer.

5. The if t.text: guard

The line immediately above, self.metrics_add_output_characters(len(t.text)), already dereferences t.text unguarded, so the guard provides no protection against None. It only filters the empty string. Note also that whitespace-only text such as three spaces is truthy and will be reported, while the block below uses t.text.strip() == "" for its own emptiness check. Two different definitions of empty within the same function. Suggest if t.text.strip(): for consistency.

6. Test coverage

No tests accompany this change. The PR body reports tests/test_params.py (1 passed), which does not exercise this code path, plus syntax and JSON/TOML parse checks. Those confirm the file compiles, not that the metric is emitted.

rime_tts/tests/test_metrics.py already has an ExtensionTester harness that subscribes to the metrics data channel and asserts on nested fields. That is a natural home for a case asserting a request-metrics event is emitted with the expected request_id and text. A multi-chunk case would also pin down the granularity question in item 2. Please run the full rime_tts suite rather than test_params.py alone.

7. Conventions

Commit and PR title follow the conventional-commits format correctly. The branch name dev/tts_request does not match the documented type/short-description pattern in AGENTS.md, which calls for a type prefix from the commit type list and hyphen separation, e.g. feat/tts-request-metrics. No action needed on an open PR, just noting it for future branches.

The patch-level bump is defensible for internal telemetry, though this does add a new outbound emission. No api schema change appears necessary since rime_tts declares api.interface and inherits from the base, but please confirm the base interface covers the new metrics event.

Nothing here is hard to address. Item 1 is the one I would resolve before merge: the reordering is a two-line move, and the version constraint needs to reflect the dependency the code now has.

[github-actions]

Review

Tight, well-scoped change: one call site, version bumped consistently in both manifest.json and pyproject.toml. A few things worth resolving before merge, mostly around ordering and semantics.

1. Hard dependency on an unmerged change (blocking)

send_tts_request_metrics has no definition anywhere in this repository — I grepped the full tree and it only appears at the new call site. It comes from TEN-framework/ten_ai_base#118, which the description notes is still open. ten_ai_base is not vendored here, so I could not verify the method signature or its argument semantics.

Consequence if this lands first: every request_tts with non-empty text raises AttributeError, gets swallowed by the generic except Exception at the bottom of request_tts, and is converted into a NON_FATAL_ERROR TTS error — so synthesis silently stops working rather than failing loudly. Worth confirming ten_ai_base#118 merges and the 0.7.58 release is published before this one, and that CI for this PR actually resolves against 0.7.58 rather than a cached 0.7.

2. Metrics call sits in front of the audio send path

await self.send_tts_request_metrics(...)
await self.client.send_text(t)

Two effects from this ordering:

• Any failure in metrics reporting means the text never reaches Rime. A telemetry problem becomes a synthesis outage, funneled through the generic handler as a vendor-agnostic non-fatal error that will be hard to attribute.
• It inserts an await point on the critical path before the send, which counts against TTFB.

Suggest moving it after send_text, and/or isolating it so it can't propagate:

await self.client.send_text(t)
try:
    await self.send_tts_request_metrics(...)
except Exception as e:
    self.ten_env.log_warn(f"Failed to report TTS request metrics: {e}")

Metrics should never be able to block audio.

3. Per-send vs per-request semantics

The title says "per-request metrics" but the implementation is per-send. request_tts is invoked once per text chunk for streaming input — the surrounding code makes this explicit, with t.request_id != self.current_request_id gating the new-request branch and t.text_input_end marking the end. Chunks 2..N reuse the same request_id, so a multi-chunk turn emits several request metrics with the same ID and different timestamps.

Is that intended? If the consumer counts requests or treats request_time_ms as the request start, this over-counts and reports the wrong start time for every chunk after the first. If you want one report per request, the natural place is inside the existing new-request branch, where self.request_start_ts is already set. If per-send is intended, the field name request_time_ms reads misleadingly.

4. Time source

int(time.time() * 1000) introduces a second time source — the extension already tracks self.request_start_ts = datetime.now() and uses it to compute durations. Reusing that value (when the intent is request start) keeps the two consistent and avoids a new import. Also note wall-clock time is not monotonic; if anything downstream subtracts these values to derive latency, an NTP step can produce negative or inflated numbers.

5. Minor

• The if t.text: guard passes whitespace-only text, which the code below treats as empty (t.text.strip() == "" on the text_input_end path). A whitespace chunk would report a request that produces no audio. if t.text and t.text.strip(): would match existing behavior.
• The guard is also inconsistent with metrics_add_output_characters(len(t.text)) on the line directly above, which is unguarded. Not a bug, just worth being deliberate about which of the two is right.
• Other call sites in this file defensively use self.current_request_id or "" for request IDs; the new call passes t.request_id raw.
• Pinning ten_ai_base to 0.7.58 when 72 of 73 extensions use 0.7 is unusual (only spatius_avatar_python pins a patch, at 0.7.40). Fine if it's a deliberate floor for the new API, but since ten_ai_base is a system package shared per-app, it would help to confirm the resolver treats 0.7 as a compatible range that 0.7.58 satisfies, rather than producing a conflict when rime_tts is graphed alongside other extensions.

6. Test coverage

No test accompanies the change. tests/test_metrics.py already has exactly the harness needed — an ExtensionTester that inspects emitted metrics data and a @patch("rime_tts.extension.RimeTTSClient") mock — so asserting the request metric is emitted is a small addition. Given the ambiguity in point 3, a test that sends a two-chunk streaming request and asserts how many request metrics come out would pin down the intended contract and catch the missing-method failure from point 1 in CI rather than at runtime. The testing notes mention only test_params.py and syntax/parse checks, none of which exercise this code path.

Nothing here is difficult to address — the ordering fix and a semantics decision on point 3 are the substantive ones.

[github-actions]

Review

Tight, well-scoped change — 7 lines of logic, version bumps in both manifest.json and pyproject.toml kept in sync, and commits follow the repo's conventional-commit rules. Most of my feedback is about semantics and merge ordering rather than the code itself.

Merge ordering (blocking)

send_tts_request_metrics does not exist anywhere in this repo — I grepped all .py and .json files and got zero hits. It is provided solely by ten_ai_base 0.7.58 (TEN-framework/ten_ai_base#118), which the description notes is still open.

The failure mode if this lands first is worth spelling out, because it is quiet rather than loud. The new await sits inside request_tts's try, so the resulting AttributeError is swallowed by the broad except Exception at the bottom, converted into a NON_FATAL_ERROR, and reported via send_tts_error. TTS would stop producing audio on every request while the extension keeps reporting itself as merely non-fatally degraded. Please hold this until #118 is released, and ideally confirm 0.7.58 is actually published before merge.

Metrics failure can break synthesis

Related, and true even after #118 lands: this await is unguarded inside the main try. If the metrics call raises for any reason — transport hiccup, schema change, downstream collector issue — it is indistinguishable from a synthesis failure and takes the audio down with it. Telemetry should not be load-bearing for the audio path:

if t.text:
    try:
        await self.send_tts_request_metrics(...)
    except Exception as e:
        self.ten_env.log_warn(f"Failed to report TTS request metrics: {e}")

Alternatively, guarantee non-raising in the base method — but the guard here is cheap and local.

Per-send vs per-request

The title says "per-request metrics" but the implementation reports per send. request_tts is invoked once per text chunk within a request — that is precisely what the if t.request_id != self.current_request_id branch and the text_input_end flag exist to track. So a single request_id streamed as five chunks emits five metrics events, each with a distinct request_time_ms.

The description ("report each Rime WebSocket text send") suggests this is deliberate, and the fix(tts): remove rime request final flag commit suggests it was already iterated on. If so, worth confirming the downstream consumer aggregates by request_id rather than assuming one event per request — otherwise streamed requests will be over-counted. If one event per request was the intent, this belongs inside the new-request branch alongside self.request_start_ts = datetime.now().

Reported before the send succeeds

The call precedes await self.client.send_text(t), so a request is recorded even when the send immediately raises ModuleVendorException and never reaches Rime. Whether that is right depends on what the metric counts: attempts or successful dispatches. If it is meant to measure work actually sent to the vendor, move it after send_text. If it is deliberately an attempt counter, a one-line comment saying so would save the next reader the same detour.

Inconsistent notion of "empty"

The if t.text: guard uses truthiness, but 20 lines below the same function tests emptiness with t.text.strip() == "". Whitespace-only text (" ") therefore reports a metric while empty text does not — two different definitions of empty in one function. if t.text.strip(): would align them.

Minor: the guard also diverges from the unguarded metrics_add_output_characters(len(t.text)) directly above it, which happily records zero characters. Not a bug, just an inconsistency a reader will pause on.

Version pin tightening

manifest.json narrows ten_ai_base from "0.7" to "0.7.58". That makes this the outlier: 72 extensions use "0.7" and only spatius_avatar_python pins a patch (0.7.40). If tman treats this as a floor it is fine and correctly expresses the new requirement; if it is exact-match, rime gets frozen at 0.7.58 and will silently miss future base fixes — and docs/ai/L1/07_gotchas.md already warns that tman install replaces system deps when newer versions exist. Worth confirming which semantics apply.

Smaller notes

• import time adds a second time source to a file that already imports and uses datetime for timing (self.request_start_ts = datetime.now(), and the TTFB computation derived from it). int(datetime.now().timestamp() * 1000) gets the same epoch-ms without the new import. Wall-clock is the right choice here — this is an absolute timestamp, not a duration, so time.monotonic() would be wrong.
• No tests accompany the change, and the description reports only test_params.py (1 test) — while this touches the metrics path that tests/test_metrics.py (141 lines) covers. Please run the full rime suite. A focused assertion is cheap: gradium_tts_python/tests/test_state_machine.py:431 mocks metrics_add_output_characters with a MagicMock, and the same pattern would pin down both call count and request_text for a multi-chunk request — which is exactly the per-send behavior worth locking in.

Summary

The mechanics are sound and the change is appropriately small. Before merge I would want the upstream dependency released, the metrics await guarded so telemetry cannot break audio, and the per-send emission confirmed as intended. The empty-text and timestamp points are polish.