Echo fallback request ids on 8.3

[goldyfruit]

Fixes #464.

Backports fallback request-id echoing onto the 8.3 line.

If a fallback ping carries fallback_request_id in data or context, the pong returns it in both places. Legacy pings keep the same shape. A data-level id wins even when it is an empty string, so concurrent fallback collectors do not accidentally fall through to a different context id.

Checked with:

PYTHONPATH=. PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 python -m pytest -q test/unittests/test_fallback_skill.py
python -m ruff check ovos_workshop/skills/fallback.py test/unittests/test_fallback_skill.py

[coderabbitai]

📝 Walkthrough

Walkthrough

The _handle_fallback_ack method in FallbackSkill was refactored to extract fallback_request_id from either message data or context, conditionally echoing it into the emitted pong's data and context. Unused List import was removed. New unit tests validate both the new echo behavior and preserved legacy shape.

Changes

Fallback request id echo

Layer / File(s)	Summary
Extract and echo fallback_request_id in pong reply ovos_workshop/skills/fallback.py	Removes unused List import and refactors _handle_fallback_ack to extract fallback_request_id from message data or context, conditionally including it in the emitted pong's data and context while preserving legacy skill_id/can_handle fields.
Tests for fallback ack request id echo test/unittests/test_fallback_skill.py	Adds _make_skill() helper and tests covering request id echo via data, via context, and legacy shape when no request id is present.

Estimated code review effort: 2 (Simple) | ~10 minutes

Sequence Diagram(s)

sequenceDiagram
  participant CoreBus
  participant FallbackSkill
  participant Reply

  CoreBus->>FallbackSkill: ping message (data/context with fallback_request_id?)
  FallbackSkill->>FallbackSkill: extract fallback_request_id from data or context
  FallbackSkill->>FallbackSkill: check can_answer
  alt fallback_request_id present
    FallbackSkill->>Reply: emit pong with fallback_request_id in data and context
  else no fallback_request_id
    FallbackSkill->>Reply: emit legacy pong with skill_id and can_handle only
  end
  Reply->>CoreBus: ovos.skills.fallback.pong

Loading

Related issues: #464 - Echo fallback request ids in pongs

Suggested labels: enhancement, tests

Suggested reviewers: JarbasAl

🐰 A ping comes in with an id to trace,
The pong now echoes it back into place,
No more lost replies in the fallback race,
Legacy skills keep their same old face.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The code echoes fallback request ids in pong replies and preserves legacy behavior when no id is present.
Out of Scope Changes check	✅ Passed	The changes stay focused on fallback reply handling and its tests, with no unrelated scope visible.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately and concisely describes the main change: echoing fallback request IDs in the 8.3 backport.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

ovos_workshop/skills/fallback.py (1)

113-127: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value

Correctly implements request-id echo with legacy fallback preserved.

Logic matches the PR objective: echoes fallback_request_id from data or context into both data and context of the pong, and leaves the shape unchanged when absent.

One minor nit: using or to chain the lookups means a falsy-but-present fallback_request_id (e.g., empty string) in data would silently fall through to context. Unlikely in practice given request ids are non-empty strings, but message.data.get(...) if ... is not None else message.context.get(...) would be more precise if that ever becomes a concern.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ovos_workshop/skills/fallback.py` around lines 113 - 127, The request-id echo
logic in the fallback pong should preserve a falsy-but-present
fallback_request_id from message.data instead of falling through to
message.context. Update the lookup in the fallback handler that builds data and
context so it distinguishes “missing” from “present but empty,” while keeping
the existing behavior of copying the ID into both payloads in the fallback
response emitted by the fallback method.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@ovos_workshop/skills/fallback.py`:
- Around line 113-127: The request-id echo logic in the fallback pong should
preserve a falsy-but-present fallback_request_id from message.data instead of
falling through to message.context. Update the lookup in the fallback handler
that builds data and context so it distinguishes “missing” from “present but
empty,” while keeping the existing behavior of copying the ID into both payloads
in the fallback response emitted by the fallback method.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e13afc0a-7332-43a9-8f1b-a172b7bbd015

📥 Commits

Reviewing files that changed from the base of the PR and between 72993f3 and e67ec8c.

📒 Files selected for processing (2)

• ovos_workshop/skills/fallback.py
• test/unittests/test_fallback_skill.py