Skip to content

Performance improvement - #112

Open
sainijit wants to merge 32 commits into
intel-retail:mainfrom
sainijit:performance-improvement
Open

sainijit wants to merge 32 commits into
intel-retail:mainfrom
sainijit:performance-improvement

Conversation

@sainijit

@sainijit sainijit commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

PR Checklist

  • Added label to the Pull Request for easier discoverability and search
  • Commit Message meets guidelines as indicated in the URL https://github.com/intel-retail/voice-enabled-interactions/blob/main/CONTRIBUTING.md
  • Every commit is a single defect fix and does not mix feature addition or changes
  • Unit Tests have been added for new changes
  • Updated Documentation as relevant to the changes
  • All commented code has been removed
  • If you've added a dependency, you've ensured license is compatible with repository license and clearly outlined the added dependency.
  • PR change contains code related to security
  • PR introduces changes that breaks compatibility with other modules (If YES, please provide details below)

What are you changing?

  1. Enable Benchmark: Enabling Benchmark for kiosk #87
  2. Performance Improvement: Improve Latency in kiosk #89

sainijit and others added 9 commits September 7, 2026 21:15
Adopts a set of measured, low-risk latency improvements for the voice
ordering pipeline, using two real customer recordings for benchmarking
and kiosk-voice-lab-main as a reference for technique comparison.

- fix: KIOSK_CORE_TTS_VOICE stale "Ryan" (pre-Kokoro-migration) causing
  100% silent TTS failures; fixed to am_michael
- feat: pre-synthesized, cached, non-committal "opener" phrase played
  while the agent's tool-call is still generating (fills ~1.6-2s of
  dead air instead of removing it)
- feat: adaptive end-of-turn endpointing — commit early when the
  transcript already reads as a finished sentence, fail closed to the
  fixed 1.5s timeout otherwise
- feat: TTS speech normalization for prices/times (₹169, 8 AM) so
  Kokoro speaks them instead of reading raw symbols/digits
- feat: phrase-level TTS streaming (release on commas, not just
  sentence ends) + new price_guard module correcting (never blocking)
  a hallucinated order total against the authoritative tool-result total
- perf: ASR device moved to NPU (ties with GPU in isolation, frees GPU
  for ovms-llm/text-to-speech under load); evaluated and reverted
  distil-whisper/distil-small.en due to a confirmed NPU state-corruption
  bug on variable-length calls
- feat: optional Silero VAD (default off) and optional skip of the
  empty final tail-chunk ASR call (default off), both available for
  further A/B testing
- fix: AnalyzerClient always sends an explicit `language` field so it
  can genuinely be left unset for English-only ASR checkpoints
- feat: latency instrumentation (endpoint_wait_ms, voice_to_voice_ms,
  voice_to_voice_informative_ms) matching the last-word-to-first-audio
  methodology used in kiosk-voice-handoff.pdf

Investigated and explicitly not adopted (documented rationale in
docs/performance-improvements-2026-09.md): continuous rolling ASR at
shorter chunk intervals (reintroduces previously-fixed Whisper
hallucination/diarization bugs), and an in-process OpenVINO GenAI LLM
pipeline as a replacement for OVMS-HTTP (benchmarked at parity, ~786ms
vs ~799ms median, with real feature regressions and no measurable win).

42 new unit tests added (all passing); full existing suite re-run with
no regressions. Companion TTS Kokoro backend and audio-analyzer tuning
changes are in the edge-ai-libraries repo (microservices/), committed
separately.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Measured follow-up to 7b67bf7, benchmarked against a working
kiosk-voice-lab-main replay as the reference implementation.

Kept (measured win):
* MCP tool-schema compaction. Tool docstrings document their return
  payload for humans, but that prose never steers tool selection or
  argument filling and was re-sent on every LLM round-trip.
  compact_tool_description() truncates at the first Returns:/Raises:/
  Yields:/Example:/Note: heading, keeping the summary and the whole
  Args: block. Source docstrings are untouched; only the LLM-facing
  copy is trimmed. 1,627 -> 1,199 tokens across the 12 kiosk tools.
  Agent turn 2,506 -> 2,277 ms median (-229 ms, -9.1%), with
  non-overlapping min/max ranges. The win lands on LLM#1, the
  prefill-dominated call, as predicted.
* AGENT_MAX_TOKENS 192 -> 128, sized from the longest legitimate
  outputs (84-token category listing, 56-token multi-item tool call).
  Bounds the free-run case only; does not shorten a normal turn.
* Silero VAD on by default, so turn timers start at the true end of
  speech rather than when an absolute RMS gate happens to trip.

Reverted (measured, no win):
* OVMS --max_num_batched_tokens 4096 -> 8192. Sound hypothesis, but
  no improvement in either an isolated turn (2,306 vs 2,277 ms) or a
  6-turn replay (llm median 5,869 vs 5,951 ms) -- both inside noise.
  Compaction already pulls static prefill back under 4096, so the
  larger budget is redundant.

Bug fixes found while measuring:
* SileroVAD accepted unsupported sample rates. Silero v5 is 8k/16k
  only; another rate builds a valid ONNX session and fails later at
  inference inside the decoder LSTM. The existing fail-open guard
  wraps only the constructor, so sessions crashed mid-turn instead of
  falling back. Enabling the flag by default turned this latent bug
  live for 24 kHz Kokoro audio. The rate is now validated in
  __init__, raising ValueError that the guard downgrades to RMS VAD.
  6-turn 24 kHz replay: 0/6 turns before, 6/6 after.
* conversation_replay_benchmark hard-coded a 16 kHz session rate
  while the TTS backend decides the real rate, so every turn was
  rejected under Kokoro. It now reads the rate from the WAV header.

Docs: corrected an earlier false claim that both stacks run int4 --
.env pins int8 here and /v3/models confirms it, so precision is part
of the gap. Also corrected the "2 LLM calls doubles the work"
analysis: LLM#2 measures ~11 ms (0.4%) because of the templated-reply
shortcut, so cost is overwhelmingly one prefill-bound call.

Testing: 18 new unit tests. Full suite run twice on the same box,
with changes stashed and applied, gave identical totals
(17 failed / 103 passed / 3 skipped / 32 errors) -- no regressions.
Those failures are a host venv numpy fault reproducing on the
untouched commit, not a baseline to accept.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ss fixes in the agent paths

LATENCY

kiosk_core/analyzer_client.py always sends an explicit `diarization` value.
It previously omitted the field when false, so the analyzer fell through to
its config default (on) and every preview chunk — flushed while the customer
is still speaking, and whose speaker attribution is never consumed — paid
full Pyannote diarization plus ECAPA enrollment. Together with the analyzer
change in edge-ai-libraries@3dc1a8d, ASR total drops 881ms -> 445ms and the
final chunk on the critical path from ~500ms to ~220ms.

Honouring the flag removed the accidental side effect that enrollment used to
piggyback on those preview chunks, so _flush_chunk now diarizes the first
chunk long enough to enroll (DIARIZATION_ENROLL_MIN_SECONDS) while the
conversation has no reference voice yet. A scope is only marked enrolled when
a segment comes back is_primary=True: the analyzer emits is_primary=False on
every segment while enrollment is still deferred, so keying off the presence
of the field would end priming before any reference voice existed.

MEASUREMENT

voice_to_voice_ms was under-reporting by ~315ms. _t_first_audio was stamped
when a sentence was handed to the TTS worker, not when audio existed; it is
only a genuine audio stamp when the opener emits it, and the opener is
disabled. It is now stamped in the TTS worker when sentence 1's WAV is
written. Confirmed on a live UI turn: voice_to_voice_ms and
voice_to_voice_informative_ms now agree (1135.5ms), where they previously
differed by the whole first-sentence synthesis.

CORRECTNESS (found in review of the in-flight agent work)

- directive_mode._execute now reports which tools actually COMMITTED writes,
  separately from which were called. Returning None after a partial success
  ("add a burger, remove the fries" where the add lands and the remove
  fails) made chat() fall back to the tool-calling path, which re-interprets
  the same utterance and applied the add a second time. A committed-then-
  failed turn can no longer fall back; it speaks an honest partial failure.
  A generation error now settles the dispatch task instead of orphaning it.

- _force_confirm called call_tool directly, bypassing the _mcp_fn wrapper
  that injects dry_run. A speculative draft — which runs on a partial,
  still-changing preview transcript with the real user_id — could therefore
  really confirm the customer's order mid-utterance. It now forwards
  _speculative_ctx, and the whole recovery path is skipped for drafts.

- Speculative drafts no longer write into the real conversation's ADK
  session (derived `::spec` session id) or its shared _CartState. Both would
  have left the following genuine turn believing the items were already
  added — the model replays that conclusion instead of re-calling the tool,
  so the customer is told the order was placed while the DB has nothing.

- reply_templates.speak() only speaks "your cart is empty" when the envelope
  positively indicates success and the payload is literally null. unwrap()
  collapses four cases to None, including JSONDecodeError, so a decode
  failure previously told a customer with a live cart that it was empty —
  and skip_summarization meant the model never got to correct it.

- _finalize_run's client cleanup no longer raises AttributeError on a
  partially-constructed session; cleanup must not fail a completed turn.

- _enrolled_scopes is bounded like _consecutive_rejections instead of
  growing one entry per conversation forever.

configs/text-to-speech: clear_storage_on_startup must be false now that the
service runs 2 uvicorn workers — the wipe runs once per worker process
against shared storage, so a late-starting or respawned worker would delete
sessions another worker is serving.

Measured (median of 4, tier-B voice E2E, live stack):
  ASR                  881ms -> 445ms
  time to first audio          ~950ms
  voice-to-voice (live UI)     1135ms, now honest
Tests: 125 passed, 1 skipped.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Condenses nine rounds of latency work into a briefing document: headline
before/after, per-token cost of each pipeline stage measured on our own
hardware (LLM 27.5ms/token, ASR 165ms/chunk, TTS 210ms + ~9-19ms/char),
and a stage-by-stage decomposition of the 1,135ms voice-to-voice figure
from a live ordering turn that reconciles with those unit costs to within 2%.

Includes the correctness work alongside the speed work, since several of
the fixes closed ways the kiosk could have made a false statement about an
order, and those are the changes most worth defending.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds the intel-retail/performance-tools submodule (pinned) used by
'make benchmark' to orchestrate voice-to-voice benchmark runs.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ark vocabulary alignment

Summary of changes bundled in this commit (branch: performance-improvement):

Voice pipeline / latency:
- Continuous ASR streaming to audio-analyzer's /v1/realtime WebSocket
  (kiosk_core/realtime_analyzer_client.py, new), gated off by default via
  KIOSK_CORE_ANALYZER_STREAMING_ENABLED.
- Adaptive endpoint completeness shortcut tuning (config.py, docker-compose.yml,
  .env.example) so the real ASR commit wins the race against the shortcut
  instead of forcing a full-utterance re-transcribe every turn.
- audio_session.py: wall-clock latency instrumentation
  (post_speech_gap_ms, voice_to_voice_post_endpoint_ms, final_flush_wait_ms,
  etc.), trailing-silence trimming, speculative TTS cache-key fix.
- Fixed a real bug found in review: continuous-streaming _streaming_active
  never re-checked liveness after a successful connect, so a mid-session
  WebSocket drop silently froze ASR for the rest of the session with no
  fallback. Now backed by a property that consults
  RealtimeAnalyzerClient.is_alive() and falls back to the file-based
  AnalyzerClient the same way a failed initial connect already does.
- Fixed a docker-compose default that contradicted its own comment
  (KIOSK_CORE_ADAPTIVE_FLUSH_PAUSE_SECONDS shipped at 0.50 despite the
  comment explaining it needed to be 0.30 to win the completeness-shortcut
  race).

ADK vs. non-ADK ordering agent comparison:
- plugins/kiosk/ordering_agent_without_adk.py (new): simulates tool-calling
  via direct function calls instead of Google ADK, for latency comparison.
- rag-service/api/agent_endpoints.py: new /api/v1/agent/chat-no-adk endpoint.
- tests/benchmarks/agent_latency_benchmark_adk.py,
  agent_latency_benchmark_no_adk.py, replay_all_conversations_adk.py,
  replay_all_conversations_no_adk.py (new): hardcoded, side-by-side
  latency/correctness benchmark scripts.
- docs/reports/ordering-agent-adk-vs-directive-2026-09.md (new): A/B writeup
  (562/562 conversations, 546/546 turns pass, 0 guard-corrections both paths).
- docs/reports/kiosk-comparison-report.md, kiosk-lab-report.md,
  kiosk-vei-report.md (new): lab-vs-this-app hardware/latency comparisons.

Benchmark tooling:
- Sample_data/conversation.jsonl (new): single-file conversation source for
  v2v_scripted_conversation_benchmark.py / make benchmark, replacing the
  hardcoded SCRIPTS dict as the default (legacy named scripts still available
  via --script).
- tests/benchmarks/perf-tools-orchestrator/benchmark_smart_kiosk_v2v.py
  (new, relocated out of the performance-tools submodule): this script was
  previously an untracked file living inside the submodule, which would
  vanish for anyone else cloning this repo and running the submodule update
  step -- moved into the app repo so it is actually version-controlled.
  Makefile's benchmark target updated accordingly.
- requirements.txt: removed a redundant/risky pip VCS install line pointing
  at performance-tools@main -- nothing imports it as a package, and it
  duplicated the pinned submodule with an unpinned ref.
- benchmark-vocabolary.txt (new): shared cross-team KPI-terminology spec.
- tests/benchmarks/v2v_fixture_benchmark.py, conversation_replay_benchmark.py:
  added kpi_vocabulary/CX-band output aligned to that spec, including a fix
  so Endpointing delay falls back to post_speech_gap_ms instead of
  reporting null when a run uses --explicit-end-mark (mic-release path).
- kiosk-ui: PipelineFlow.tsx's retired TTFA chip replaced with a Processing
  chip; VoiceToVoiceTable.tsx's V2V (pipeline) column/labels renamed to
  Processing latency to match the shared vocabulary.

Cleanup:
- Removed tests/benchmarks/results/*.json (33 previously-committed ad-hoc
  tuning-run dumps) and gitignored results/ and tests/benchmarks/results/
  going forward -- these are regenerated output, not source.
- Removed a stray duplicate smart-kiosk-assistant/.gitmodules, zips, a PDF,
  and local .m4a recordings that had accumulated untracked in the working
  tree (not previously committed to history; cleaned from disk only).
- Fixed a Sample_data .gitignore pattern mismatch (sample2.mp4 vs the real
  sample_1.mp4/sample_2.mp4 filenames -- neither large sample video was
  actually being ignored).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
# Conflicts:
#	smart-kiosk-assistant/docker-compose.yml
#	smart-kiosk-assistant/requirements.txt
…ent env vars

Fix 23 unit test failures introduced by two production changes landed in the
prior commit:

1. `_streaming_active` becoming a property (backed by
   `__streaming_active_flag`) instead of a plain attribute. Several
   `tests/unit/*.py` files build `BaseAudioSession` via `__new__()` to bypass
   `__init__`, so they never got the new backing attributes. Fixed by adding
   `realtime_client`/`_streaming_active` (and, where needed, `_lock`) to the
   affected test constructors.

2. `_scope_is_enrolled()`-gated authoritative analyzer rejection. Tests
   written before scope-enrollment tracking existed asserted "reject on
   analyzer verdict" without ever marking their synthetic scope enrolled, so
   they now exercise the (correct) never-enrolled fallback path instead.
   Fixed by calling `_mark_scope_enrolled()` in the tests that are explicitly
   about the enrolled/authoritative-rejection regression, and by adding
   `agent_session_id` to the remaining `_make_session()` helpers so
   `_scope_is_enrolled()` has something to look up at all.

Also fixed a related production bug in `_finalize_run`'s cleanup path:
`self.realtime_client` was read directly (unlike `client`/`tts_client`/
`agent_client`, which are all read via `try/except`), so an AttributeError
here could escape a "never raise" cleanup path for any partially-built
session. Wrapped it in `getattr(self, "realtime_client", None)` to match the
other three.

Finally, added the 11 docker-compose.yml env vars that
`test_compose_env_vars_covered_by_env_example` flagged as undocumented
(`AGENT_DIRECTIVE_MODE`, `ASR_DEVICE`, the adaptive-flush/ASR-trim/VAD/
speculative-execution flags, `METRICS_COLLECTOR_REGISTRY`,
`TEXT_TO_SPEECH_WORKERS`) to `.env.example` with the same defaults compose
already falls back to.

Result: tests/unit now 140 passed, 1 skipped (was 23 failed, 117 passed, 1
skipped). tests/functional -m tier1 now only fails on a pre-existing,
environment-level beartype/pytest circular-import issue that is unrelated to
this branch (reproduced identically on origin/main).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

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

Critical deployment, streaming, VAD, validation, and test-collection issues remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 11 High severity · 3 Medium severity

Open (14)
What changed in this PR

Performance improvements across the kiosk voice pipeline, including endpointing, TTS, ASR, streaming, validation, and observability.

Changes:

  • Adds adaptive endpointing, VAD, opener caching, and latency metrics.
  • Updates TTS/ASR deployment configuration and speech normalization.
  • Adds phrase streaming, price validation, and regression tests.
File Description
smart-kiosk-assistant/​tests/​unit/​test_skip_empty_final_flush.py Tests final ASR flush skipping.
smart-kiosk-assistant/​tests/​unit/​test_silero_vad.py Tests Silero VAD behavior.
smart-kiosk-assistant/​tests/​unit/​test_sentence_gate_pregrounded.py Tests pre-grounded streaming safety.
smart-kiosk-assistant/​tests/​unit/​test_sentence_gate_phrase_splitting.py Tests comma-level phrase streaming.
smart-kiosk-assistant/​tests/​unit/​test_endpoint_completeness.py Tests adaptive endpoint detection.
smart-kiosk-assistant/​rag-service/​tests/​test_price_guard.py Tests order-total validation.
smart-kiosk-assistant/​plugins/​kiosk/​price_guard.py Validates order totals.
smart-kiosk-assistant/​plugins/​kiosk/​ordering_agent.py Adds phrase splitting and streaming safeguards.
smart-kiosk-assistant/​kiosk_core/​tts_client.py Applies speech normalization before synthesis.
smart-kiosk-assistant/​kiosk_core/​speech_normalizer.py Adds deterministic speech conversions.
smart-kiosk-assistant/​kiosk_core/​silero_vad.py Adds the Silero VAD wrapper.
smart-kiosk-assistant/​kiosk_core/​pipeline_latency.py Adds latency and flush metrics.
smart-kiosk-assistant/​kiosk_core/​config.py Adds performance feature flags and defaults.
smart-kiosk-assistant/​kiosk_core/​audio_session.py Implements endpointing, opener caching, and flush changes.
smart-kiosk-assistant/​kiosk_core/​analyzer_client.py Sends explicit ASR language values.
smart-kiosk-assistant/​docs/​performance-improvements-2026-09.md Documents performance changes and benchmarks.
smart-kiosk-assistant/​docker-compose.yml Configures TTS builds and runtime settings.
smart-kiosk-assistant/​configs/​text-to-speech/​config.yaml Selects the Kokoro TTS runtime.
smart-kiosk-assistant/​configs/​audio-analyzer/​config.yaml Selects NPU ASR execution.
smart-kiosk-assistant/​.env.example Documents runtime configuration settings.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread smart-kiosk-assistant/configs/audio-analyzer/config.yaml Outdated
Comment thread smart-kiosk-assistant/configs/text-to-speech/config.yaml
Comment thread smart-kiosk-assistant/docker-compose.yml Outdated
Comment thread smart-kiosk-assistant/docker-compose.yml
Comment thread smart-kiosk-assistant/docker-compose.yml
Comment thread smart-kiosk-assistant/plugins/kiosk/price_guard.py Outdated
Comment thread smart-kiosk-assistant/tests/unit/test_endpoint_completeness.py
Comment thread smart-kiosk-assistant/kiosk_core/audio_session.py Outdated
Comment thread smart-kiosk-assistant/kiosk_core/audio_session.py
Comment thread smart-kiosk-assistant/kiosk_core/silero_vad.py Outdated
sainijit and others added 2 commits September 21, 2026 22:17
Verified all 9 findings against the current code before fixing (one, the
"pulled TTS image lacks Kokoro" pair, turned out already handled by the
Makefile's unconditional local text-to-speech build — fixed via the build
context path instead, see below).

1. configs/audio-analyzer/config.yaml + docker-compose.yml + .env.example:
   ASR device default was NPU while ACCEL_MOUNT_PATH's safe/portable default
   stays /dev/null, so ASR failed to start on CPU/GPU-only hosts following
   the documented .env. Reverted the default to CPU across all three (config
   default, compose env-var fallback, .env.example); NPU is opt-in via
   ASR_DEVICE=NPU + a real ACCEL_MOUNT_PATH.

2. docker-compose.yml + Makefile: audio-analyzer/text-to-speech build
   contexts used `../../edge-ai-libraries/...`, but
   docs/user-guide/get-started/build-from-source.md documents cloning
   edge-ai-libraries one level up (sibling of smart-kiosk-assistant/, i.e.
   `../edge-ai-libraries`). Fixed both build contexts and the Makefile's
   matching echo strings/comment.

3. docker-compose.yml: added KIOSK_CORE_TTS_MODEL: kokoro alongside the
   existing KIOSK_CORE_TTS_VOICE override — kiosk_core.config.DEFAULT_TTS_MODEL
   otherwise falls back to "qwen-tts", which the Kokoro-backed TTS service's
   matches_model_name() would not recognise.

4. plugins/kiosk/price_guard.py: _TOTAL_TOOLS omitted get_order and
   remove_from_order even though the MCP server returns an authoritative
   total for both, letting a stale total slip past the guard for either
   tool. Added both to the allowlist.

5. .github/workflows/kiosk-functional-tests.yml: tests/unit/ is deliberately
   excluded from pytest.ini's default testpaths (functional only, to keep
   tier1/2/3 marker selection unambiguous), but nothing was invoking it in
   CI either -- it silently never ran. Added an explicit "Run unit tests"
   step to the Tier 1 job (no Docker/ML models needed, same as Tier 1
   functional).

6. kiosk_core/audio_session.py: two bugs found in the same file --
   a. `_render_opener`'s cache key was (text, model, voice, language),
      omitting `instructions` even though it's passed to synthesis. The
      first opener rendered for a given text/model/voice/language locked in
      whatever instructions were used first; later requests with different
      instructions silently got the wrong speaking style replayed. Added
      instructions to the key (and updated its type annotation/comment) --
      the digest calculation already iterates the whole key tuple so no
      other change was needed.
   b. `_chunk_has_speech` was only set in the "already speech_started"
      per-frame branch; the very first speech frame takes the
      "not self._speech_started" branch and `continue`s before ever
      reaching that assignment, so a short utterance could leave the flag
      False and get dropped by the empty-final-flush skip. Set
      `_chunk_has_speech`/`_unconfirmed_speech_pending` in the speech-start
      branch too.

7. kiosk_core/silero_vad.py: FRAME was a fixed 512-sample class constant
   regardless of sample_rate, so the 8kHz path (which needs a 256-sample hop
   + 32-sample context per Silero v5's spec) fed the model the wrong input
   length. Replaced FRAME with a FRAME_SIZES map and a per-instance
   self.frame_size derived from sample_rate in __init__; updated
   tests/unit/test_silero_vad.py's one direct FRAME reference to match.

Verified: tests/unit (140 passed, 1 skipped) and tests/functional -m tier1
(same pre-existing beartype/pytest environment failures as before this
change, confirmed unrelated) both unchanged/green after all fixes. Also
py_compile + yaml.safe_load + `make -n build` sanity-checked every changed
file.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

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

One or more issues must be addressed before approval.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 5 High severity

Open (5)
Resolved since last review (14)

Comment thread .gitmodules
Comment thread smart-kiosk-assistant/configs/text-to-speech/config.yaml
Comment thread smart-kiosk-assistant/kiosk-ui/src/hooks/useVoiceSession.ts Outdated
Comment thread smart-kiosk-assistant/rag-service/api/agent_endpoints.py
Comment thread smart-kiosk-assistant/tests/unit/test_speculative_tts_cache_key.py
- kiosk-ui/gradio: stop hardcoding tts_model=speecht5 so the server-side
  DEFAULT_TTS_MODEL (kokoro) applies as intended.
- useVoiceSession.ts: add acquireMicWithTimeout() to dispose a
  late-resolving getUserMedia() stream when the acquisition timeout has
  already fired, preventing a leaked live microphone track.
- agent_endpoints.py: gate tool_call_detail (raw tool kwargs/results) to
  speculative=True requests only, in both the AgentChatResponse path and
  the raw /chat/stream path, since the endpoint has no auth and the port
  is published in docker-compose.yml.
- audio_session.py: preserve decimal points between digits in
  _normalize_sentence_for_cache_key so ₹169.50 and ₹16950 no longer
  collide on the same TTS cache key, while still stripping a genuine
  sentence-ending period.
- tests: add regression coverage for the decimal-point cache-key fix.

.gitmodules gitlink-missing finding was investigated and found to be
stale/non-reproducible (gitlink for performance-tools is already
committed); no change needed.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

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

Unresolved security, correctness, lifecycle, and benchmark issues must be addressed before approval.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 High severity

Open (1)
Resolved since last review (5)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Include confirm_order in authoritative total handling

smart-kiosk-assistant/​plugins/​kiosk/​price_guard.py:57

confirm_order is missing from _TOTAL_TOOLS, even though the MCP confirm_order tool returns the confirmed order including its authoritative total. After a successful direct confirmation, this guard therefore leaves state.total unset and a fallback reply can still speak a hallucinated total. Add confirm_order here and cover that path.

Comment thread smart-kiosk-assistant/rag-service/api/agent_endpoints.py Outdated
Copilot review on PR intel-retail#112: /chat-no-adk (routes to
OrderingAgentWithoutADK, can place/confirm real orders through the
same MCP tools) was decorated on the same router main.py mounts
whenever ORDERING_AGENT_ENABLED is set (the default), so it was
reachable in every normal deployment despite its docstring claiming
otherwise.

- agent_endpoints.py: only register the /chat-no-adk route when
  AGENT_BENCHMARK_ENDPOINTS_ENABLED=true (default: false); a normal
  deployment never mounts it now.
- docker-compose.yml / .env.example: document the new flag, off by
  default.
- tests/benchmarks/agent_latency_benchmark_no_adk.py: force the flag
  to true when it brings the stack up via `make up`, so the one
  script that legitimately needs this route keeps working unattended.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

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

Unresolved critical order-integrity and speculative-execution safety issues block approval.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 3 High severity

Open (3)
Resolved since last review (1)
Previously missed (1)

In code that hasn't changed since last review

Medium severity p95 calculation omits the maximum tail sample

smart-kiosk-assistant/​tests/​benchmarks/​llm_direct_ttft_bench.py:112

This is not a standard p95 calculation: with the default 8 runs it selects index 6 (the 7th value), so the reported p95 can omit the maximum tail sample and understate the latency target. Use a nearest-rank/ceil-based percentile (or statistics.quantiles) so the benchmark's p95 actually reflects the slow tail.

Comment thread smart-kiosk-assistant/plugins/kiosk/price_guard.py
Comment thread smart-kiosk-assistant/rag-service/api/agent_endpoints.py
Comment thread smart-kiosk-assistant/rag-service/api/agent_endpoints.py
sainijit and others added 5 commits September 22, 2026 00:34
- Relocate the v2v benchmark orchestrator into the performance-tools
  submodule (benchmark-scripts/), mirroring order-accuracy/take-away's
  layout so it can bring the stack up the same way. Keep a tracked
  backup + README under tests/benchmarks/perf-tools-orchestrator/, and
  add `make install-benchmark-orchestrator` to auto-restore it after
  `make update-submodules` wipes the untracked submodule copy.
- Add `sync-metrics-into-results`, `parse-qmassa-metrics`,
  `consolidate-metrics`, and `plot-metrics` Makefile targets that chain
  off `make benchmark`, all scoped to ./results (never the repo root)
  to avoid consolidate_multiple_run_of_metrics.py's filename-substring
  matching misparsing unrelated source files.
- Default V2V_RUNS to 1 (was 12) so `make benchmark` runs the sample
  conversation once by default.
- Fix qmassa GPU-metrics reliability:
  - `make down` no longer clears ./metrics/ (only `make up` does, as a
    prerequisite before the next run) -- it was deleting the
    just-collected qmassa file before consolidate-metrics/plot-metrics
    ever ran, since the benchmark orchestrator calls `make down`
    internally right after each measured run.
  - Add `stop_grace_period: 30s` to metrics-collector in
    docker-compose.yml, giving qmassa more time to flush/close its
    JSON document before Docker SIGKILLs it during full-stack teardown.
  - Add tests/benchmarks/repair_qmassa_json.py as a defense-in-depth
    safety net: detects and repairs a qmassa JSON file truncated
    mid-write (tracking bracket depth to find the last complete
    top-level state entry, then truncating and re-closing there), wired
    into `sync-metrics-into-results` ahead of the upstream parser.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- audio_session.py: require _chunk_has_speech for the "real content" flush
  branch and add a peek-before-commit shortcut in _flush_chunk, fixing a
  race where a final flush's redundant commit(wait=True) would burn the
  full 2.5s timeout even though an async completion had already landed
  (measured: turn 2's final_flush_wait_ms dropped from ~2500ms to ~1ms).
- config.py / .env.example: lower DEFAULT_ENDPOINT_STABLE_SECONDS from
  0.2 to 0. The two-confirmation stability window needed two preview
  round-trips (~90-220ms each) to land inside the 0.15-1.1s shortcut
  window, which the quiet-mode preview cadence (0.15s) only barely
  allowed -- measured firing rate was 1/4 turns (25%). Single-
  confirmation mode (trust the first complete-looking snapshot) raised
  this to a consistent 3-4/4 turns (75-100%) across repeated
  full-conversation runs and 7/10 turns across
  tests/benchmarks/v2v_fixture_benchmark.py (real recorded speech),
  with zero transcript regressions in all sampled runs. Documented the
  real trade-off observed: firing this early sometimes beats
  DEFAULT_ADAPTIVE_FLUSH_PAUSE_SECONDS's own real commit to the punch,
  shifting cost into final_flush_wait_ms on some turns -- net effect
  across full-conversation benchmarks was still a clear win (median
  v2v ~1996ms -> ~1310-1360ms).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The ovms_ov_cache named volume was created root:root 0755 by Docker on
first use, but the ovms-llm image runs as non-root uid/gid 5000 (user
"ovms"), which could never write into it -- OVMS logged "Cache
directory /tmp/ov_cache is not writable" on every startup and silently
recompiled the full model IR each time instead of hitting the cache,
causing LLM cold-start TTFT spikes.

Adds a one-shot ovms-cache-init service (busybox, runs as root,
chown -R 5000:5000 /tmp/ov_cache) gated via
depends_on/service_completed_successfully before ovms-llm starts.

Verified: cache dir is now ovms:ovms 0755, the "not writable" warning
is gone, /tmp/ov_cache holds ~2.2GB of compiled .blob/.cl_cache files
after first boot, and a second restart came back healthy in ~2s
instead of recompiling.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…endoring

Replaces the byte-for-byte vendored copy of performance-tools'
vlm_metrics_logger.py (kiosk_core/vlm_metrics_logger.py) with a pip
dependency straight from the same repo:

  git+https://github.com/intel-retail/performance-tools.git@main#subdirectory=benchmark-scripts

performance-tools is already a build-time dependency for benchmarking
elsewhere in this project, so this removes the manual-sync burden of a
vendored copy without pulling in any extra transitive dependencies --
that subdirectory's setup.py declares no install_requires, so pip only
installs the single vlm_metrics_logger module, not performance-tools'
own dev requirements.txt (pandas/matplotlib/etc.).

- requirements.txt: add the git+https dependency with rationale
- Dockerfile: add git to apt-get install (required for pip to fetch a
  git URL during image build)
- audio_session.py: import from top-level 'vlm_metrics_logger' instead
  of 'kiosk_core.vlm_metrics_logger'
- config.py: update comments to match
- Deleted the now-unused vendored kiosk_core/vlm_metrics_logger.py

Verified: kiosk-core image rebuilds cleanly, module imports and writes
vlm_application_metrics_*.txt correctly standalone, and a full 'make
benchmark' run confirms end-to-end (Transactions: 4, cross-check ok).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
sainijit and others added 6 commits September 22, 2026 21:34
…ubmodule

- Remove tests/rec1_16k.wav, tests/rec2_16k.wav, tests/fixtures/sample_speech_16k.wav
  (real-recording binary fixtures) and their functional references; update
  v2v_fixture_benchmark.py to require --fixture explicitly instead of
  defaulting to bundled recordings, and test_silero_vad.py to use a
  synthetic non-silent signal for state/context-threading tests (the one
  test that specifically needed real speech was removed, since that can't
  be meaningfully replicated synthetically)
- Remove 10 unused tests/benchmarks/ scripts not referenced by the
  make benchmark flow and not imported by any kept file
  (agent_latency_benchmark_adk.py, agent_latency_benchmark_no_adk.py,
  conversation_replay_benchmark.py, lab_v2v_fixture_benchmark.py,
  llm_direct_ttft_bench.py, model_matrix_benchmark.sh,
  replay_all_conversations*.py, tts_latency_benchmark.py)
- Remove tests/benchmarks/perf-tools-orchestrator/ recovery-backup folder:
  benchmark_smart_kiosk_v2v.py is now committed directly inside the
  performance-tools submodule, so the untracked-file recovery mechanism
  (and the Makefile's install-benchmark-orchestrator target/fallback) is
  no longer needed
- Bump the performance-tools submodule pointer to the commit that adds
  benchmark_smart_kiosk_v2v.py
- Chain sync-metrics-into-results/consolidate-metrics onto `make benchmark`
  so hardware counters (qmassa/cpu/memory/npu) land in ./results/
  automatically instead of only in ./metrics/
- Remove gradio_app.py and benchmark-vocabolary.txt (already deleted in
  the working tree prior to this commit)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…models.sh

silero_vad.onnx (~2.2MB) was committed directly to git. Since it's baked
into the kiosk-core image at build time the same way YOLO26n's IR is baked
into queue-service (COPY . . in the Dockerfile), fetch it the same way:
via setup_models.sh, with a pinned SHA256 (matches the official
snakers4/silero-vad v5 release) verified before and after download so a
corrupt/partial fetch or unexpected upstream change fails loudly instead
of silently landing in the image.

Verified: `./setup_models.sh --skip-ovms --skip-queue` downloads and
checksum-verifies the file from scratch; tests/unit/test_silero_vad.py
(16 tests) passes against the freshly downloaded model.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Three fixes, found and verified sequentially via make benchmark:

1. plugins/kiosk/directive_mode.py: <check_price> directive prompted
   the model to say NOTHING before the tag (unlike <act>, which speaks
   prose first), serializing ~1s of dead air with nothing to hide
   behind -- _execute_price() is a pure in-memory dict lookup, not an
   MCP/network call. Now requires a short prose filler ("Let me
   check.") before the tag, mirroring <act>'s pattern.

2. tests/benchmarks/v2v_fixture_benchmark.py: replay_fixture()'s
   chunk-push loop slept after every chunk, including the last one,
   before signaling end-of-stream -- unlike production's kiosk-ui
   useVoiceSession.stop(), which force-flushes and signals end
   immediately on button release. Added one-chunk lookahead so the
   sleep is skipped after the final chunk, making the benchmark
   accurately reflect production's push-to-talk timing instead of
   masking real ASR round-trip time in background sleep.

3. configs/audio-analyzer/config.yaml + .env(.example) +
   docker-compose.yml: the final/persisted-commit ASR pool ran
   whisper-small on CPU (~500-570ms/call) versus the preview pool's
   distil-small.en on GPU (~40-60ms/call), on the (path-dependent, and
   here invalid) assumption that the final round-trip "overlaps the
   silence wait either way." That's only true on the natural
   silence-timeout path -- false on the explicit-end-mark/push-to-talk
   path that kiosk-ui's real stop() button uses, where the full ASR
   round-trip is pure unhidden latency gating turn start. Switched the
   final pool to the same distil-small.en/GPU combo already proven
   corruption-free (English-only checkpoint is fine: this deployment
   is English-only via KIOSK_CORE_ASR_LANGUAGE=en). ASR_DEVICE default
   changed CPU->GPU accordingly (never NPU with this checkpoint -- see
   corruption-bug comments preserved in config.yaml).

   This surfaced a companion bug in audio-analyzer's realtime endpoint
   (fixed separately in edge-ai-libraries): the final-commit path
   forwarded session.language unconditionally, which crashes
   openvino_genai for English-only checkpoints. Without that fix this
   config change breaks all transcripts.

Net result (make benchmark, ground-truth v2v, n=12 turns):
median ~1200-1300ms -> ~700-980ms, p95 ~1188ms. Target of ~800ms met.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ith the v2v-latency work

1. Removed both "Voice-to-Voice per Turn" tables (one inside PipelineFlow's
   AI Inference Pipeline panel, one as a standalone VoiceToVoiceTable below
   the Performance KPIs section) -- redundant duplicate displays. Deleted
   the now-unused VoiceToVoiceTable.tsx component entirely. The V2V/V2V p95
   summary chips already in PipelineFlow's header are unaffected (those
   aren't the per-turn table).

2. PipelineFlow's AI Inference Pipeline stage chips now reflect the exact
   vocabulary tracked during the v2v-latency work instead of generic
   cumulative/round-trip figures:
     - ASR: asr.last_word_to_transcript_ms (real compute latency, last
       spoken word -> transcript ready) instead of the "all chunks summed"
       total -- answers "how much ASR latency is taking".
     - LLM: agent.ttft_ms (time to first token) instead of cumulative model
       time across every round-trip in the turn.
     - TTS: time to first audio -- the TTS-only slice of
       wall.time_to_first_audio_ms remaining after the LLM TTFT above,
       instead of cumulative synth time for every segment.
   Updated stage tooltips and the footnote to match.

3. Added a V2V Latency card (customer's last word -> first sound out) to
   ExecutiveKpis' Performance KPIs section, alongside the existing E2E/ASR/
   LLM/TTS cards.

Verified with a full `npm run build` (tsc -b && vite build) -- no errors.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Built and pushed intel/kiosk-ui, intel/audio-analyzer,
intel/text-to-speech, intel/kiosk-core, intel/rag-service,
intel/queue-service, and intel/identity-service all tagged
:latest to Docker Hub. Point a fresh `make init-env` at the
same tag so REGISTRY=true pulls the images that were just
published instead of the stale 2026.2.0 tag.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
sainijit and others added 2 commits September 24, 2026 07:33
- price_guard.py: add confirm_order to _TOTAL_TOOLS. It returns the same
  authoritative order total as confirm_active_order, but was missing from
  the guarded set, letting a hallucinated total slip past mismatched()/
  validate_reply() on that path.

- agent_endpoints.py: reject speculative=true on /chat-no-adk.
  OrderingAgentWithoutADK's chat() accepts `speculative` but never
  implements the dry-run contract — its order-mutation path
  (directive_mode.run_turn) calls place_order/remove_from_order/
  confirm_active_order with no dry_run flag at all, so a speculative
  request on this benchmark-only route could persist a real order
  mutation. Reject it with 400 instead; speculative turns must go
  through the production /chat agent, which correctly forces dry_run
  via _mcp_fn/_speculative_ctx.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
sainijit and others added 6 commits September 24, 2026 09:23
…ence-interval overrides

Investigated moving queue-service's YOLO26 detector from NPU to GPU to
fix CPU contention (queue-service+rtsp-streamer were pinning ~7.8/16
cores, starving the voice pipeline). GPU + VA-memory zero-copy did cut
queue-service CPU by ~50% as expected, but a same-box A/B (3 turns each,
back-to-back) showed it consistently added ~200-350ms to ovms-llm ttft
and up to ~1.9s to voice-to-voice latency from sharing the GPU with the
LLM, even with inference_interval raised to shed GPU cycles. Reverted
runtime config to NPU (no functional change to defaults), but exposed
QUEUE_VA_MEMORY and QUEUE_INFERENCE_INTERVAL as documented env overrides
(matching the existing QUEUE_DEVICE pattern) alongside the measured
tradeoff, so this doesn't need to be re-investigated from scratch if
CPU contention becomes the bigger problem again on different hardware.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…face endpoint wait

ExecutiveKpis and PipelineFlow independently re-derived the same trace's
ASR/LLM/TTS numbers using different fields (ExecutiveKpis: raw cumulative
asr.ms/agent.llm.ms/tts.ms; PipelineFlow: critical-path
last_word_to_transcript_ms/ttft_ms/TTFA slice), so the same turn showed
different numbers in each panel despite a header comment claiming they
were 'the SAME source ... so the two panels always agree'. Extracted the
extraction logic into a shared kiosk-ui/src/utils/turnLatency.ts module
(extractLatencies/formatLatency/percentile) and made both panels import
from it, and fixed a secondary decimal-precision mismatch on V2V
formatting (toFixed(2) vs toFixed(1)) that made an identical number
display differently (1.36s vs 1.4s).

Also added an 'Endpoint wait' chip to PipelineFlow: voice_to_voice_ms
includes real, customer-felt non-compute wait (endpoint_wait_ms +
post_speech_gap_ms + final_flush_wait_ms -- trailing-silence dwell,
mic-release reaction time, ASR flush-queue drain) that was already
recorded in the trace but never surfaced, so V2V never visibly
reconciled with the sum of the ASR/LLM/TTS chips.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…int-wait burst

Push-to-talk withheld all audio until Stop, so the whole utterance's
frame processing (VAD, etc.) had to run in one CPU burst right after
Stop, inflating post_speech_gap_ms by hundreds of ms to over a second.

Now push-to-talk uploads in the same small tuning.chunkSeconds (0.5s)
slices conversation mode already used, keeping the backend's per-frame
pipeline caught up in near real time while the customer is still
talking. This does not change when ASR actually fires: chunk_seconds/
silence_timeout_seconds are still maxed out for single-chunk sessions
and the adaptive pre-warm flush is still disabled, so the backend only
buffers these uploads and transcribes the full, uncut utterance in one
Whisper call at the explicit end-of-stream signal (Stop), exactly as
before.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…lies stop contradicting themselves

_run_turn accumulated every text fragment ADK emitted across a whole
turn with a naive "".join(reply_parts), including any text the model
generated BEFORE deciding to call a tool. On compound/two-item requests
(e.g. "one peri peri fries and one onion rings") the model sometimes
emits a clarifying/confirming sentence first, then calls update_order,
then emits the real grounded answer — and both got glued together with
no separator, producing self-contradicting replies like "Would you
like one Peri Peri Fries and one Onion Rings? Got it. ... total is now
₹357" or worse, a false "is unavailable" followed immediately by
offering the same item back.

Clear reply_parts whenever a function_call is observed, so only text
generated after the LAST tool call in the turn — the actual grounded
final answer — survives. Text before/around a tool call is discarded
outright rather than spoken.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
docker-compose.yml references ${KIOSK_CORE_VLM_METRICS_ENABLED:-true} but
.env.example never documented it, failing
test_compose_env_vars_covered_by_env_example in CI.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…inal-flush skip regression

1. tests/unit/test_silero_vad.py needs the real bundled ONNX model, but
   setup_models.sh (82ec327) stopped git-tracking
   kiosk_core/models/silero_vad.onnx in favour of downloading it on demand.
   The Tier 1 job never ran setup_models.sh (that's Tier 2's job, and pulls
   in far more than this ~2.2 MB file), so those tests started failing
   with NoSuchFile as soon as the model left git. Added a small, cached,
   checksum-verified download step for just this file before the unit
   test step.

2. BaseAudioSession._process_frame_stream's final-tail-chunk branch had a
   real regression: the fallback 'elif chunk_frames and self._speech_started'
   branch unconditionally treated any non-realtime session's silent final
   chunk (_streaming_active=False, e.g. browser push-to-talk over POST or
   file-replay) as skipped, regardless of
   DEFAULT_SKIP_EMPTY_FINAL_FLUSH_ENABLED's value. With the flag off (the
   default), the final chunk should always be enqueued even when it's pure
   trailing silence -- this restores that behavior and only actually skips
   when skip_final_flush is true.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.

4 participants