fix(review): harden relay_overrides parser + polish (PR #178 review)
Addresses verified findings from the /review-pr pass on PR #178. H1 — relay_overrides values are now type-checked - Allowlist of recognised knobs (`video_strength`, `video_window_scale`, `audio_strength`, `audio_window_scale`, `audio_epsilon`) with explicit float-coercion. Numeric strings ("10") coerce silently; non-numeric values (e.g. "ten") now raise a ValueError naming the offending knob and showing a minimal correct example — instead of a TypeError deep inside `build_segments` arithmetic, or a SUCCESS render at upstream defaults that polluted the A/B iteration log. H2 — malformed JSON no longer silently falls back to defaults - `json.JSONDecodeError`, non-dict JSON (arrays, strings, bare numbers, null), and non-numeric values for recognised knobs all now raise `ValueError` with operator-actionable messages. The only silent fallback is the explicit empty/whitespace case — "use upstream defaults" is the operator's intent there. - Typo'd keys (e.g. `viedo_strength`) log a warning naming the unknown keys + the recognised set, then drop the typo. Previously they were silently retained in the cleaned dict but never queried by `build_segments`, indistinguishable from a default render. Architectural side-effect — shrinks the diff vs vendored upstream - `_parse_relay_overrides` extracted into a stdlib-only sibling module `_relay_overrides.py`. `ltx_director.py` shrinks closer to upstream baseline (one import + one alias instead of a 30-line function body) and is easier to diff against `e81223a`. The parser is also now unit-testable without ComfyUI's `comfy_api` runtime. L5 — tests added (31 cases, all passing) - New `tests/forks/__init__.py` + `tests/forks/test_whatdreamscost_v1_3_2_relay_overrides.py`. - Covers every confirmed failure mode (H1 + H2), the happy path, the comment-key convention, the unknown-key warning, and an allowlist- consistency assertion that fails loudly if `build_segments` ever grows a knob without the parser being updated. - Test module uses `importlib.util.spec_from_file_location` so it can load the parser without walking through the package hierarchy (which would trigger the `comfy_api` import inside `ltx_director.py`). L3 — `_build_line` promoted to public `build_line` - `scripts/sync_to_dev.py` renames the private helper. The underscore alias is kept for one release cycle as a deprecation slot. - `scripts/sync_to_dev_audio.py` switches to `_dev.build_line()`. No behaviour change; chat-report shape is bit-identical to before. L2 — `source_ref` clarified in pin metadata - `forks/whatdreamscost_koolook/versions/v1_3_2/UPSTREAM_PIN.yaml` and `forks/forks_manifest.yaml` both now spell out that `e81223a` is the post-v1.3.2 README-only commit on upstream's `main`, not the v1.3.2 release tag itself. Code is byte-identical to the tagged release; the pin differs only in trailing docs. L1, L4 are explicitly left alone (L1 is dead upstream cruft, kept for minimum-diff; L4 is helper duplication tracked for a separate follow-up issue when a third fork lands). Verification - ruff check . → clean - pytest → 204 passed (173 prior + 31 new parser tests) - tools/preflight_release.py --skip manager-meta → all PASS (static AST extraction still picks up LTXDirector from the fork) - scripts/sync_to_dev_audio.py --dry-run → two-line chat report still emits via the renamed `build_line` helper Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.7
parent
54c035afd0
commit
3ffb7b75aa
@@ -42,7 +42,7 @@ entries:
|
|||||||
- id: "whatdreamscost_koolook_v1_3_2_ltx_director"
|
- id: "whatdreamscost_koolook_v1_3_2_ltx_director"
|
||||||
name: "WhatDreamsCost Koolook v1.3.2 LTX Director subset"
|
name: "WhatDreamsCost Koolook v1.3.2 LTX Director subset"
|
||||||
source_repo: "https://github.com/WhatDreamsCost/WhatDreamsCost-ComfyUI.git"
|
source_repo: "https://github.com/WhatDreamsCost/WhatDreamsCost-ComfyUI.git"
|
||||||
source_ref: "v1.3.2"
|
source_ref: "v1.3.2 (pinned to post-tag README-only commit on main)"
|
||||||
pinned_commit: "e81223a2add687555a6371e311b325880bf7c3c9"
|
pinned_commit: "e81223a2add687555a6371e311b325880bf7c3c9"
|
||||||
external_checkout:
|
external_checkout:
|
||||||
relative_path_from_forks_root: "whatdreamscost-v1.3.2-koolook"
|
relative_path_from_forks_root: "whatdreamscost-v1.3.2-koolook"
|
||||||
|
|||||||
@@ -1,5 +1,10 @@
|
|||||||
source_repo: "https://github.com/WhatDreamsCost/WhatDreamsCost-ComfyUI.git"
|
source_repo: "https://github.com/WhatDreamsCost/WhatDreamsCost-ComfyUI.git"
|
||||||
source_ref: "v1.3.2"
|
# source_ref names the release. pinned_commit is the latest `main` SHA
|
||||||
|
# at the time the fork was cut — a README-only update sitting one commit
|
||||||
|
# beyond the actual `v1.3.2` release tag on upstream's history. Code is
|
||||||
|
# byte-identical to the tagged release; the SHA differs because upstream
|
||||||
|
# pushed a docs touch-up afterwards.
|
||||||
|
source_ref: "v1.3.2 (pinned to post-tag README-only commit on main)"
|
||||||
pinned_commit: "e81223a2add687555a6371e311b325880bf7c3c9"
|
pinned_commit: "e81223a2add687555a6371e311b325880bf7c3c9"
|
||||||
external_checkout_relative_to_forks_root: "whatdreamscost-v1.3.2-koolook"
|
external_checkout_relative_to_forks_root: "whatdreamscost-v1.3.2-koolook"
|
||||||
upstream_tracking_relative_to_forks_root: "whatdreamscost-main-upstream"
|
upstream_tracking_relative_to_forks_root: "whatdreamscost-main-upstream"
|
||||||
|
|||||||
@@ -0,0 +1,126 @@
|
|||||||
|
# SPDX-License-Identifier: GPL-3.0-or-later
|
||||||
|
"""
|
||||||
|
Pure-stdlib parser for the LTXDirector node's ``relay_overrides`` widget.
|
||||||
|
|
||||||
|
Lives in its own module (not in ``ltx_director.py``) for two reasons:
|
||||||
|
|
||||||
|
1. ``ltx_director.py`` imports ``comfy_api`` at module load, so it can't be
|
||||||
|
imported outside a running ComfyUI process. This module imports only
|
||||||
|
stdlib + logging, so unit tests can exercise the parser directly.
|
||||||
|
2. Keeping this Koolook addition out of ``ltx_director.py`` shrinks the
|
||||||
|
diff against vendored upstream — easier to spot the two upstream
|
||||||
|
modifications in ``ltx_director.py`` when comparing against the pinned
|
||||||
|
upstream commit.
|
||||||
|
|
||||||
|
Parser contract — see :func:`parse_relay_overrides`.
|
||||||
|
"""
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import json
|
||||||
|
import logging
|
||||||
|
from typing import Optional
|
||||||
|
|
||||||
|
log = logging.getLogger(__name__)
|
||||||
|
|
||||||
|
|
||||||
|
# Allowlist of recognised override keys and their coercion type. Anything
|
||||||
|
# outside this set (other than ``_``-prefixed comment keys) is dropped
|
||||||
|
# with a warning so typos like ``viedo_strength`` don't silently render
|
||||||
|
# at upstream defaults — they'd look identical to a clean default render
|
||||||
|
# in the iteration log, polluting any A/B comparison.
|
||||||
|
RELAY_OVERRIDE_KEYS: dict[str, type] = {
|
||||||
|
"video_strength": float,
|
||||||
|
"video_window_scale": float,
|
||||||
|
"audio_strength": float,
|
||||||
|
"audio_window_scale": float,
|
||||||
|
"audio_epsilon": float,
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
def parse_relay_overrides(s: str) -> Optional[dict]:
|
||||||
|
"""Parse the LTXDirector ``relay_overrides`` multiline-string input.
|
||||||
|
|
||||||
|
The widget value is JSON. Empty / whitespace input means "use upstream
|
||||||
|
defaults" — the only silent fallback. Anything else that doesn't
|
||||||
|
decode to a JSON object of numeric knob values is treated as
|
||||||
|
operator error and raises ``ValueError`` so the render fails fast
|
||||||
|
instead of silently producing a default render the maintainer would
|
||||||
|
later mistake for a real "the override had no effect" result.
|
||||||
|
|
||||||
|
Args:
|
||||||
|
s: The widget's text body.
|
||||||
|
|
||||||
|
Returns:
|
||||||
|
A dict of validated ``{knob_name: float}`` overrides, or ``None``
|
||||||
|
when the input is empty/whitespace (meaning "use upstream
|
||||||
|
defaults"). The dict only contains keys from
|
||||||
|
:data:`RELAY_OVERRIDE_KEYS`; unknown keys are dropped after
|
||||||
|
logging a warning.
|
||||||
|
|
||||||
|
Raises:
|
||||||
|
ValueError: when the input is non-empty but doesn't parse as a
|
||||||
|
JSON object, or when a recognised knob's value can't be
|
||||||
|
coerced to ``float``. The error message names the offending
|
||||||
|
knob and shows a minimal correct example so the operator can
|
||||||
|
fix the widget without consulting the upstream paper.
|
||||||
|
"""
|
||||||
|
if not s or not s.strip():
|
||||||
|
return None
|
||||||
|
|
||||||
|
try:
|
||||||
|
opts = json.loads(s)
|
||||||
|
except (json.JSONDecodeError, TypeError) as exc:
|
||||||
|
raise ValueError(
|
||||||
|
"relay_overrides input is not valid JSON "
|
||||||
|
f"({type(exc).__name__}: {exc}). Empty the field to use "
|
||||||
|
'upstream defaults, or paste a JSON object like '
|
||||||
|
'{"video_strength": 10.0}.'
|
||||||
|
) from exc
|
||||||
|
|
||||||
|
if not isinstance(opts, dict):
|
||||||
|
raise ValueError(
|
||||||
|
"relay_overrides must be a JSON object, got "
|
||||||
|
f"{type(opts).__name__}. Empty the field to use upstream "
|
||||||
|
'defaults, or paste a JSON object like '
|
||||||
|
'{"video_strength": 10.0}.'
|
||||||
|
)
|
||||||
|
|
||||||
|
cleaned: dict[str, float] = {}
|
||||||
|
unknown: list[str] = []
|
||||||
|
|
||||||
|
for key, value in opts.items():
|
||||||
|
if key.startswith("_"):
|
||||||
|
# `_`-prefixed keys are reserved as inline JSON comments —
|
||||||
|
# carried in the widget value, ignored at parse time.
|
||||||
|
continue
|
||||||
|
if key not in RELAY_OVERRIDE_KEYS:
|
||||||
|
unknown.append(key)
|
||||||
|
continue
|
||||||
|
|
||||||
|
coerce = RELAY_OVERRIDE_KEYS[key]
|
||||||
|
try:
|
||||||
|
cleaned[key] = coerce(value)
|
||||||
|
except (TypeError, ValueError) as exc:
|
||||||
|
raise ValueError(
|
||||||
|
f"relay_overrides[{key!r}] must be a number, got "
|
||||||
|
f"{value!r} ({type(value).__name__}). Example: "
|
||||||
|
f'{{"{key}": 10.0}}'
|
||||||
|
) from exc
|
||||||
|
|
||||||
|
if unknown:
|
||||||
|
log.warning(
|
||||||
|
"[PromptRelay] relay_overrides has unknown keys (typo?): %s. "
|
||||||
|
"Recognised keys: %s. Use `_`-prefixed keys for inline "
|
||||||
|
"comments.",
|
||||||
|
sorted(unknown),
|
||||||
|
sorted(RELAY_OVERRIDE_KEYS),
|
||||||
|
)
|
||||||
|
|
||||||
|
if not cleaned:
|
||||||
|
return None
|
||||||
|
|
||||||
|
log.info("[PromptRelay] Loaded relay_overrides from node input: %s", cleaned)
|
||||||
|
return cleaned
|
||||||
|
|
||||||
|
|
||||||
|
__all__ = ["RELAY_OVERRIDE_KEYS", "parse_relay_overrides"]
|
||||||
@@ -311,37 +311,11 @@ def _convert_to_latent_lengths(pixel_lengths, temporal_stride, latent_frames):
|
|||||||
return result
|
return result
|
||||||
|
|
||||||
|
|
||||||
def _parse_relay_overrides(s):
|
# KOOLOOK PATCH (issue #177, 2026-05-28):
|
||||||
"""KOOLOOK PATCH (Koolook issue #177, 2026-05-28):
|
# The relay_overrides parser lives in a stdlib-only sibling module so it
|
||||||
Parse Prompt Relay per-stream overrides from a JSON string typed into the
|
# can be unit-tested without ComfyUI. See _relay_overrides.py for the
|
||||||
LTX Director's `relay_overrides` widget. No disk I/O — values live entirely
|
# parser contract (allowlist, type coercion, fail-fast on malformed JSON).
|
||||||
inside the workflow JSON and travel with it.
|
from ._relay_overrides import parse_relay_overrides as _parse_relay_overrides
|
||||||
|
|
||||||
Returns a dict or None. None == upstream behaviour.
|
|
||||||
|
|
||||||
Supported keys (see prompt_relay.build_segments):
|
|
||||||
video_strength (float, default 1.0) — multiplies attn2 penalty
|
|
||||||
video_window_scale (float, default 1.0) — scales flat-top width
|
|
||||||
audio_strength (float, default 1.0) — multiplies audio_attn2 penalty
|
|
||||||
audio_window_scale (float, default 1.0) — scales flat-top width (audio)
|
|
||||||
audio_epsilon (float, None) — independent sigma for audio path
|
|
||||||
Underscore-prefixed keys are ignored (use for inline comments).
|
|
||||||
"""
|
|
||||||
if not s or not s.strip():
|
|
||||||
return None
|
|
||||||
try:
|
|
||||||
opts = json.loads(s)
|
|
||||||
except Exception as e:
|
|
||||||
log.warning("[PromptRelay] relay_overrides input is not valid JSON, ignoring: %s", e)
|
|
||||||
return None
|
|
||||||
if not isinstance(opts, dict):
|
|
||||||
log.warning("[PromptRelay] relay_overrides must be a JSON object, got %s. Ignoring.", type(opts).__name__)
|
|
||||||
return None
|
|
||||||
cleaned = {k: v for k, v in opts.items() if not k.startswith("_")}
|
|
||||||
if not cleaned:
|
|
||||||
return None
|
|
||||||
log.info("[PromptRelay] Loaded relay_overrides from node input: %s", cleaned)
|
|
||||||
return cleaned
|
|
||||||
|
|
||||||
|
|
||||||
def _encode_relay(model, clip, latent, global_prompt, local_prompts, segment_lengths, epsilon, relay_overrides=""):
|
def _encode_relay(model, clip, latent, global_prompt, local_prompts, segment_lengths, epsilon, relay_overrides=""):
|
||||||
|
|||||||
+14
-3
@@ -110,7 +110,7 @@ def _get_worktree_name() -> str:
|
|||||||
return REPO_ROOT.name
|
return REPO_ROOT.name
|
||||||
|
|
||||||
|
|
||||||
def _build_line() -> str:
|
def build_line() -> str:
|
||||||
"""Composes the two-piece header line consumed by the chat-report
|
"""Composes the two-piece header line consumed by the chat-report
|
||||||
convention defined in project CLAUDE.md:
|
convention defined in project CLAUDE.md:
|
||||||
|
|
||||||
@@ -118,11 +118,22 @@ def _build_line() -> str:
|
|||||||
|
|
||||||
SHA falls back to ``unknown`` if git is unreachable (we always need
|
SHA falls back to ``unknown`` if git is unreachable (we always need
|
||||||
SOMETHING in slot 1 — the line shape is part of the convention).
|
SOMETHING in slot 1 — the line shape is part of the convention).
|
||||||
Worktree name comes from ``REPO_ROOT.name`` and is always present."""
|
Worktree name comes from ``REPO_ROOT.name`` and is always present.
|
||||||
|
|
||||||
|
Public — consumed by scoped per-module wrappers like
|
||||||
|
``sync_to_dev_audio.py`` so every dev-sync variant emits the same
|
||||||
|
chat-report header.
|
||||||
|
"""
|
||||||
sha = _get_short_sha() or "unknown"
|
sha = _get_short_sha() or "unknown"
|
||||||
return f"{sha} - {_get_worktree_name()}"
|
return f"{sha} - {_get_worktree_name()}"
|
||||||
|
|
||||||
|
|
||||||
|
# Backwards-compatible alias (the function was private until the audio
|
||||||
|
# wrapper landed). Drop after the next release cycle once we're sure no
|
||||||
|
# downstream caller imports the underscore name.
|
||||||
|
_build_line = build_line
|
||||||
|
|
||||||
|
|
||||||
def trigger_restart(url: str, timeout: float = 2.0) -> tuple[bool, str]:
|
def trigger_restart(url: str, timeout: float = 2.0) -> tuple[bool, str]:
|
||||||
"""Best-effort POST/GET to ComfyUI-Manager's reboot endpoint.
|
"""Best-effort POST/GET to ComfyUI-Manager's reboot endpoint.
|
||||||
|
|
||||||
@@ -431,7 +442,7 @@ def main() -> int:
|
|||||||
# chat-report convention that consumes this output. Header first so
|
# chat-report convention that consumes this output. Header first so
|
||||||
# the maintainer's eye lands on the build identifier before the
|
# the maintainer's eye lands on the build identifier before the
|
||||||
# mechanical sync details.
|
# mechanical sync details.
|
||||||
print(_build_line())
|
print(build_line())
|
||||||
print(f"{verb} {n} entries -> {target}")
|
print(f"{verb} {n} entries -> {target}")
|
||||||
if not args.dry_run:
|
if not args.dry_run:
|
||||||
write_build_info(target, args.scope)
|
write_build_info(target, args.scope)
|
||||||
|
|||||||
@@ -148,7 +148,7 @@ def main() -> int:
|
|||||||
paths=AUDIO_PATHS,
|
paths=AUDIO_PATHS,
|
||||||
)
|
)
|
||||||
verb = "would sync" if args.dry_run else "synced"
|
verb = "would sync" if args.dry_run else "synced"
|
||||||
print(_dev._build_line())
|
print(_dev.build_line())
|
||||||
print(f"{verb} {n} entries -> {target} (dev-sync-audio)")
|
print(f"{verb} {n} entries -> {target} (dev-sync-audio)")
|
||||||
if not args.dry_run:
|
if not args.dry_run:
|
||||||
_dev.write_build_info(target, args.scope)
|
_dev.write_build_info(target, args.scope)
|
||||||
|
|||||||
@@ -0,0 +1,215 @@
|
|||||||
|
"""Tests for the LTXDirector ``relay_overrides`` widget parser.
|
||||||
|
|
||||||
|
These guard the failure modes flagged in the PR #178 verified review
|
||||||
|
(H1 — values were never type-checked; H2 — malformed JSON silently fell
|
||||||
|
back to upstream defaults and polluted A/B iteration logs). The parser
|
||||||
|
lives in
|
||||||
|
``forks/whatdreamscost_koolook/versions/v1_3_2/_relay_overrides.py`` —
|
||||||
|
deliberately stdlib-only so we can import it here without pulling in
|
||||||
|
ComfyUI's ``comfy_api`` runtime (which the rest of ``ltx_director.py``
|
||||||
|
needs and which can't run inside pytest).
|
||||||
|
"""
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import importlib.util
|
||||||
|
import json
|
||||||
|
import logging
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
|
||||||
|
# Load the parser module by file path. We avoid the normal package import
|
||||||
|
# (``from forks.whatdreamscost_koolook... import ...``) because walking
|
||||||
|
# into that package triggers ``forks/whatdreamscost_koolook/versions/
|
||||||
|
# v1_3_2/__init__.py``, which imports ``ltx_director`` → ``comfy_api`` —
|
||||||
|
# ComfyUI-only and unavailable inside pytest. The parser module itself
|
||||||
|
# is pure stdlib + logging, so loading it in isolation is fine.
|
||||||
|
_RELAY_PATH = (
|
||||||
|
Path(__file__).resolve().parents[2]
|
||||||
|
/ "forks"
|
||||||
|
/ "whatdreamscost_koolook"
|
||||||
|
/ "versions"
|
||||||
|
/ "v1_3_2"
|
||||||
|
/ "_relay_overrides.py"
|
||||||
|
)
|
||||||
|
_spec = importlib.util.spec_from_file_location(
|
||||||
|
"whatdreamscost_v1_3_2_relay_overrides", _RELAY_PATH
|
||||||
|
)
|
||||||
|
_mod = importlib.util.module_from_spec(_spec)
|
||||||
|
_spec.loader.exec_module(_mod)
|
||||||
|
|
||||||
|
RELAY_OVERRIDE_KEYS = _mod.RELAY_OVERRIDE_KEYS
|
||||||
|
parse_relay_overrides = _mod.parse_relay_overrides
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# Silent fallback: only the empty/whitespace case should return None.
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("blank", ["", " ", "\n", "\t \n"])
|
||||||
|
def test_empty_or_whitespace_means_use_upstream_defaults(blank):
|
||||||
|
assert parse_relay_overrides(blank) is None
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# Happy path: known knobs with numeric values get coerced to float.
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
def test_single_known_knob():
|
||||||
|
out = parse_relay_overrides('{"video_strength": 10.0}')
|
||||||
|
assert out == {"video_strength": 10.0}
|
||||||
|
assert isinstance(out["video_strength"], float)
|
||||||
|
|
||||||
|
|
||||||
|
def test_all_known_knobs():
|
||||||
|
payload = {
|
||||||
|
"video_strength": 2.5,
|
||||||
|
"video_window_scale": 0.7,
|
||||||
|
"audio_strength": 0.5,
|
||||||
|
"audio_window_scale": 1.2,
|
||||||
|
"audio_epsilon": 0.001,
|
||||||
|
}
|
||||||
|
assert parse_relay_overrides(json.dumps(payload)) == payload
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
"raw,expected",
|
||||||
|
[
|
||||||
|
('{"video_strength": 10}', 10.0), # int coerced to float
|
||||||
|
('{"video_strength": "10"}', 10.0), # numeric string coerced
|
||||||
|
('{"video_strength": "10.5"}', 10.5), # decimal string coerced
|
||||||
|
('{"video_strength": 1e2}', 100.0), # scientific notation
|
||||||
|
('{"video_strength": -1.5}', -1.5), # negatives flow through
|
||||||
|
],
|
||||||
|
)
|
||||||
|
def test_numeric_values_are_coerced_to_float(raw, expected):
|
||||||
|
out = parse_relay_overrides(raw)
|
||||||
|
assert out == {"video_strength": expected}
|
||||||
|
assert isinstance(out["video_strength"], float)
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# Comment keys: underscore-prefixed entries are silently dropped.
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
def test_underscore_prefixed_keys_are_dropped_silently(caplog):
|
||||||
|
raw = '{"_doc": "trial 3", "_owner": "daisy", "video_strength": 5}'
|
||||||
|
with caplog.at_level(logging.WARNING):
|
||||||
|
out = parse_relay_overrides(raw)
|
||||||
|
assert out == {"video_strength": 5.0}
|
||||||
|
# No warning was emitted for the comment keys.
|
||||||
|
assert "unknown keys" not in caplog.text
|
||||||
|
|
||||||
|
|
||||||
|
def test_only_comment_keys_returns_none():
|
||||||
|
assert parse_relay_overrides('{"_doc": "no overrides yet"}') is None
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# Unknown keys: warn-and-drop, don't silently misroute the operator's
|
||||||
|
# typo into the default-render bucket.
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
def test_unknown_key_logs_warning_and_drops_it(caplog):
|
||||||
|
raw = '{"viedo_strength": 10, "video_strength": 2.5}'
|
||||||
|
with caplog.at_level(logging.WARNING):
|
||||||
|
out = parse_relay_overrides(raw)
|
||||||
|
assert out == {"video_strength": 2.5}
|
||||||
|
assert "unknown keys" in caplog.text.lower()
|
||||||
|
assert "viedo_strength" in caplog.text
|
||||||
|
|
||||||
|
|
||||||
|
def test_only_unknown_keys_returns_none(caplog):
|
||||||
|
with caplog.at_level(logging.WARNING):
|
||||||
|
out = parse_relay_overrides('{"bogus": 1, "also_bogus": 2}')
|
||||||
|
assert out is None
|
||||||
|
assert "unknown keys" in caplog.text.lower()
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# H2 — malformed JSON / non-object JSON must raise (not silently use
|
||||||
|
# upstream defaults, which previously polluted A/B comparisons).
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
"raw",
|
||||||
|
[
|
||||||
|
'{"video_strength":10.0', # missing closing brace
|
||||||
|
'{"video_strength": 10,}', # trailing comma
|
||||||
|
'{video_strength: 10}', # unquoted key
|
||||||
|
"not json at all",
|
||||||
|
],
|
||||||
|
)
|
||||||
|
def test_malformed_json_raises(raw):
|
||||||
|
with pytest.raises(ValueError, match="not valid JSON"):
|
||||||
|
parse_relay_overrides(raw)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
"raw",
|
||||||
|
[
|
||||||
|
'[1, 2, 3]', # array
|
||||||
|
'"video_strength"', # string
|
||||||
|
"42", # number
|
||||||
|
"true", # bool
|
||||||
|
"null", # null
|
||||||
|
],
|
||||||
|
)
|
||||||
|
def test_non_object_json_raises(raw):
|
||||||
|
with pytest.raises(ValueError, match="must be a JSON object"):
|
||||||
|
parse_relay_overrides(raw)
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# H1 — a known knob with a non-numeric value must raise at parse time,
|
||||||
|
# not blow up deep inside the sampler arithmetic.
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
"raw",
|
||||||
|
[
|
||||||
|
'{"video_strength": "ten"}',
|
||||||
|
'{"video_strength": null}',
|
||||||
|
'{"video_strength": [1, 2]}',
|
||||||
|
'{"video_strength": {"nested": 1}}',
|
||||||
|
'{"audio_epsilon": "very small"}',
|
||||||
|
],
|
||||||
|
)
|
||||||
|
def test_non_numeric_known_knob_raises(raw):
|
||||||
|
with pytest.raises(ValueError, match="must be a number"):
|
||||||
|
parse_relay_overrides(raw)
|
||||||
|
|
||||||
|
|
||||||
|
def test_error_message_names_offending_knob_and_shows_example():
|
||||||
|
"""The error message must let the operator fix the widget without
|
||||||
|
consulting upstream Prompt-Relay source."""
|
||||||
|
with pytest.raises(ValueError) as exc:
|
||||||
|
parse_relay_overrides('{"video_window_scale": "narrow"}')
|
||||||
|
msg = str(exc.value)
|
||||||
|
assert "video_window_scale" in msg
|
||||||
|
assert "Example:" in msg
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# Allowlist sanity: documented keys match implementation.
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
def test_documented_knob_set_matches_implementation():
|
||||||
|
"""If anyone adds a knob in build_segments without updating the
|
||||||
|
allowlist, this test fails — preventing silent typos-disguised-as-
|
||||||
|
knobs that the H1 fix is meant to catch."""
|
||||||
|
assert RELAY_OVERRIDE_KEYS == {
|
||||||
|
"video_strength": float,
|
||||||
|
"video_window_scale": float,
|
||||||
|
"audio_strength": float,
|
||||||
|
"audio_window_scale": float,
|
||||||
|
"audio_epsilon": float,
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user