fix(gateway): key-presence precedence for quick_commands nested fallback - #67992
Conversation
Follow-up for 7354374: the quick_commands nested-fallback used a truthiness check (`qc is None`), so a present-but-empty top-level quick_commands (e.g. `quick_commands: null`) was silently replaced by gateway.quick_commands — inconsistent with the key-presence precedence session_reset and stt now use, where the fallback fires only when the top-level key is genuinely absent (not merely falsy/null). This is the exact bug class the salvaged PR NousResearch#59779 follow-up fixed for its two siblings; quick_commands was left on the old `is None` gate. Switch to `"quick_commands" not in yaml_cfg` so a present top-level value is honored even when empty, and add precedence + nested-fallback regression tests mirroring the session_reset/stt ones. Signed-off-by: sergioperezcheco <checo520@outlook.com>
Related: #67982 merged the neighboring nested-config bridge, while this live, focused patch retains a demonstrated quick_commands present-null precedence residual. |
|
To clarify against the triage note: #67982 merged the neighboring nested-config bridge, but this PR fixes a separate `quick_commands` present-null precedence edge case that #67982 does not cover. The residual bug (nested key missing → fallback ignored) still reproduces on current main. Happy to add a regression test if that would help confirm the distinct scope. |
|
Thanks for the focused regression fix. The quick_commands premise is confirmed on current main: Problems
Suggested changes
This is an automated hermes-sweeper review. |
…reaming Apply the same presence-based gate quick_commands now uses to profile_routes and streaming: a present-but-null/mistyped top-level key must not be silently replaced by the nested gateway.* form. The nested fallback fires only when the top-level key is genuinely absent. Adds precedence + fallback regression tests mirroring the existing quick_commands/session_reset/stt ones. Signed-off-by: sergioperezcheco <checo520@outlook.com>
|
Applied the presence-based gate to the sibling paths: |
Follow-up to 7354374. That commit established a key-presence precedence contract for load_gateway_config(): a present (even empty/falsy) top-level value must NOT be silently replaced by the nested gateway.* form, and it fixed session_reset and stt to use
"key" not in yaml_cfggating instead of truthiness checks. quick_commands was left on the oldqc is Nonegate, so it's the one remaining sibling that doesn't honor that contract.Concretely,
quick_commands: nullat the top level alongside a populatedgateway.quick_commandsblock causes the nested value to leak through, because yaml_cfg.get("quick_commands") returns None for both "absent" and "present-null". The fix switches the gate to"quick_commands" not in yaml_cfg, matching session_reset/stt exactly. The legitimate nested-only path (top-level key genuinely absent) still falls back normally.I added two regression tests mirroring the session_reset/stt ones: one asserting a present-null top-level blocks the nested fallback, and one sanity check that the nested fallback still fires when the top-level key is absent. I verified the first test fails on the pre-fix code and passes after, and the full tests/gateway/test_config.py suite (132 tests) stays green.