pipeline enhancement - #159
Conversation
vlm_client.py / api.py / validation_service.py / worker.py: - Externalize all VLM prompt templates into configs/prompt_config.json (new) so prompt wording/rules can be tuned without code changes or a container rebuild (file is volume-mounted read-only). Falls back to a built-in default config if the file is missing/invalid. - Add a quantity-counting rule (order servings, not piece count) to fix VLMs over-counting individual pieces (e.g. nuggets) as quantity. - Add VLMClient.warmup(), wired into api.py startup_event(), sending a dummy inference at container boot to absorb the OVMS lazy graph/kernel init penalty (~1-3.5s) before the first real user request. - Add guided/structured decoding via OpenAI-compatible response_format JSON schema (toggle: VLM_GUIDED_DECODING), producing clean minified JSON and reducing completion tokens. - Cap max_tokens via configs/prompt_config.json (default 96) instead of a fixed 512, reducing latency for the compact JSON responses this pipeline actually needs. - Add prediction_debug_logger.py (new): dedicated JSONL debug log (results/logs/vlm_predictions.log) capturing prompt, raw response, parsed output, and performance metadata per request for troubleshooting. - Thread image_filename through worker.py -> api.py -> validation_service -> vlm_client for richer debug logging. configs/orders.json: - Fix MCD-1003/MCD-1004 order manifests, which had each other's expected items swapped, causing false "detection failures" that were actually a test-data bug, not a model issue. Makefile: - benchmark: clean up stale containers (metrics-collector name conflicts) before each run instead of failing silently with all-zero results. - Default BENCHMARK_ITERATIONS 1 -> 8 so a benchmark run exercises all 4 sample images (2 cycles) instead of only the first one. ovms-service/setup_models.sh: - Register OpenVINO/Phi-3.5-vision-instruct-int8-ov and openbmb/MiniCPM-V-2_6 as supported model sources. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…curacy Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR enhances the video/VLM pipeline by tuning RTSP GStreamer parameters for stability, expanding OVMS model setup/configurability (including Hugging Face auth), and adding VLM prompt/response improvements plus debug observability (prompt config, structured decoding, warmup, and JSONL debug logs).
Changes:
- Adjust RTSP pipeline defaults (latency/timeout/retry, queue tuning, h264parse) in both take-away station worker and core runner.
- Expand
setup_models.shto support multiple model sources, validateOVMS_MODEL_NAME, and load optional HF auth tokens from app.env. - Improve dine-in VLM client behavior (square letterbox preprocessing, prompt config file, guided decoding schema, warmup) and add prediction/validation debug logging with added request metadata.
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| take-away/src/parallel/station_worker.py | Adjust RTSP defaults and persistent pipeline string (latency/timeout/drop-on-latency, queue params, h264parse). |
| take-away/src/core/pipeline_runner.py | Make RTSP latency and FPS configurable via env; update queue settings and RTSP element properties. |
| take-away/.env.example | Document optional HF token for gated model downloads. |
| ovms-service/setup_models.sh | Add supported-model registry, validate model selection, load HF tokens, and improve export error reporting. |
| ovms-service/README.md | Update model export instructions and document OVMS_MODEL_NAME behavior and HF auth. |
| dine-in/src/worker.py | Pass image filename through to validation for richer diagnostics. |
| dine-in/src/services/vlm_client.py | Add prompt config loading, guided decoding schema/max token cap, warmup call, square resize behavior, and debug logging metadata. |
| dine-in/src/services/validation_service.py | Propagate image filename into VLM inference and log validation debug records. |
| dine-in/src/services/prediction_debug_logger.py | Introduce JSONL debug logger for prediction/validation artifacts. |
| dine-in/src/api.py | Add VLM warmup during startup; include filename in single/batch validation calls. |
| dine-in/README.md | Update setup documentation to reflect model selection and optional HF auth. |
| dine-in/Makefile | Increase benchmark iterations and add cleanup of stale containers before benchmarking. |
| dine-in/configs/prompt_config.json | Add runtime-editable prompt template/config for VLM prompting and schema examples. |
| dine-in/configs/orders.json | Update example orders used by the app/benchmarks. |
| dine-in/.env.example | Change default model and document optional HF auth token. |
| .gitignore | Ignore additional results dir and .github/skills/ session artifacts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- setup_models.sh: use PIPESTATUS[0] instead of tee's exit code so
model export failures are correctly detected instead of masked by
the pipeline's success.
- ovms-service/README.md: list all models actually supported by
setup_models.sh (MiniCPM-V-4_5-int4, MiniCPM-V-2_6, Phi-3.5-vision),
not just Qwen and MiniCPM-V-4_5.
- station_worker.py: tie rtspsrc's internal timeout to the
timeout_sec parameter (in microseconds) instead of a hardcoded 5s,
so it stays consistent with caller-provided timeouts.
- prediction_debug_logger.py: gate JSONL debug logging behind
VLM_PREDICTION_DEBUG_LOG env flag (default off) to avoid unbounded
disk growth/I O in production.
- vlm_client.py: join Phi inventory items with newlines instead of
commas to match the documented {inventory_list} placeholder format.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 17 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (3)
take-away/src/parallel/station_worker.py:936
_verify_rtsp_stream_quick()hard-codeslatency=500and derives its owntimeout=value, which can drift fromPipelineConfig(e.g., if latency/timeout are tuned via config). Useself.pipeline_configvalues (and cap the rtspsrc timeout to the overall process timeout) so verification behavior matches the actual pipeline settings.
'timeout', str(timeout_sec),
'gst-launch-1.0', '-e',
'rtspsrc', f'location={self.rtsp_url}', 'protocols=tcp', 'latency=500',
f'timeout={timeout_sec * 1000000}', # rtspsrc timeout is in microseconds
dine-in/src/services/vlm_client.py:520
max_output_tokenscomes from JSON config and is used as the OpenAI-compatiblemax_tokenspayload field. If the config is edited to a non-integer (string/null/etc.), the request will fail at runtime. Coerce/validate this value and fall back toMAX_OUTPUT_TOKENSon invalid input.
# Instance-level max_tokens/response_format so configs/prompt_config.json
# controls these too (falls back to the class-level defaults above if the
# config doesn't specify them).
self.max_output_tokens = self.prompt_config.get("max_output_tokens", self.MAX_OUTPUT_TOKENS)
self.response_format = json.loads(json.dumps(self.RESPONSE_FORMAT)) # deep copy
dine-in/src/services/vlm_client.py:807
_load_prompt_config()claims a bad edit of configs/prompt_config.json can’t take the service down, but_build_prompt()assumes the loaded templates are strings and calls.format()unconditionally. If a user accidentally sets a template key tonull/non-string, this will raise and break inference. Add type-safe fallbacks to the built-in templates before calling.format().
if self.model_name.startswith("OpenVINO/Phi"):
inventory_list = "\n".join(self.inventory_items) if self.inventory_items else ""
template = cfg.get("phi", {}).get("with_inventory_template", "")
prompt = template.format(
inventory_list=inventory_list,
Address Copilot review on PR intel-retail#159: RTSP_LATENCY was read as a raw string and interpolated into a GStreamer pipeline executed with shell=True. Parse both env vars as integers with range checks and safe defaults. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 17 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
ovms-service/setup_models.sh:88
- Associative-array value access uses an unquoted subscript, which can break when OVMS_MODEL_NAME contains characters that undergo word-splitting/globbing. Quote the subscript to make the lookup robust.
MODEL_SOURCES["${OVMS_MODEL_NAME_ENV}"]="${SUPPORTED_MODEL_SOURCES[${OVMS_MODEL_NAME_ENV}]}"
ovms-service/setup_models.sh:77
- Associative-array lookup uses an unquoted subscript, which can trigger word-splitting/globbing if OVMS_MODEL_NAME contains whitespace or glob characters, and can fail for unexpected values. Quote the subscript when checking membership.
This issue also appears on line 88 of the same file.
if [ -z "${SUPPORTED_MODEL_SOURCES[${OVMS_MODEL_NAME_ENV}]+x}" ]; then
dine-in/src/services/vlm_client.py:261
- _smart_resize() now upscales images that are already smaller than max_size (scale_factor > 1), increasing preprocessing cost without adding information. If the intent is letterboxing without upscaling, cap the scale factor at 1.0 and only downscale when needed.
target_size = self.max_size
scale_factor = min(target_size / original_width, target_size / original_height)
ITEP-93258: make up aborted on redeploy when the container had chowned the bind-mounted results/ directory to a UID the host user does not own. chmod now tolerates EPERM, matching the idiom already used at Makefile:605. ITEP-94280: NNCF INT8 calibration can abort with SIGSEGV inside the OpenVINO CPU plugin on NVL. Because the download, FP32 export and INT8 quantization shared one heredoc under `set -e`, the crash killed setup entirely and skipped artifact verification. INT8 now runs as an isolated step that traps the exit code, retries once with a conservative ONEDNN_MAX_CPU_ISA, discards partial IR, and degrades to FP32 instead of failing setup. frame_selector falls back to the FP32 model when the INT8 IR is absent. ITEP-91371: gpu_utilization reported 0.0 for a busy GPU. GPU samples are synthesised from qmassa output and cached, and _extract_peak_from_series silently fell back to the last sample (post-inference idle) when no sample fell inside the window, so a dead collector, a stalled collector, a clock skew and a genuinely idle GPU all produced an identical 0.0. Extraction now returns a status alongside the value, never passes an out-of-window sample off as in-window, exposes metrics_status in the API response, and logs an actionable warning. gpu_memory_utilization was hardcoded to 0.0 and is now reported as unsupported. Also tag the dine-in image as :latest. DINEIN_TAG is kept separate from TAG because TAG also selects intel/retail-benchmark:$(TAG), which has no latest tag. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…el name ITEP-93258: the container ran as a fixed uid 1000 and chowned the bind-mounted results/ to it. On hosts where the invoking user is not uid 1000 (e.g. EMT) the host user then could not chmod or remove those files, so a second `make up` failed. The entrypoint now remaps the dlstreamer user to HOST_UID/HOST_GID before dropping privileges, so results/ stays owned by the invoking user. The remap uses usermod/groupmod and still execs via `gosu dlstreamer` by name, which preserves supplementary groups - notably `video`, which GPU access depends on. Remapping to root is refused, and the whole block is skipped when HOST_UID/HOST_GID are unset, so existing deployments are unaffected. Also fixes an unrelated startup defect found while validating: the OVMS model name in .env.example lacked the precision suffix that setup_models.sh writes into ovms-service/models/config.json, so every inference request returned HTTP 404 and validation produced zero results. Verified: remap correct for unset/matching/mismatched/root HOST_UID; video group retained; files written to a bind mount are owned by the host uid; full dine-in stack healthy with a live GPU validation returning accuracy 1.0 and metrics_status gpu_utilization "ok". Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
VLM_PRECISION was int8 while OVMS_MODEL_NAME ended in "-int4". The suffix is only a directory label; the actual weight format comes from --weight-format, so setup_models.sh produced INT8 weights in a directory named "-int4". Re-running setup_models.sh overwrote a previously INT4 export with INT8, regressing dine-in benchmark mean latency from 2274 ms to 3791 ms and TPS from 12.52 to 7.74. Decode is memory-bandwidth-bound, so doubling weight bytes (3.94 GB -> 7.57 GB) costs ~1.6x. Setting int4 restores it: mean 2216 ms, TPS 12.89, accuracy 1.0 (8/8), image_max_size 448 unchanged throughout. The export now reports dtype=int4, bits=4, group_size=128 with u4 weights. Note: setup_models.sh skips export when the target directory already exists and does not compare precision, so changing VLM_PRECISION alone is a silent no-op - the stale export must be removed first. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
No description provided.