perf: remove transformer runner hot-path overhead - #427
Conversation
📝 WalkthroughWalkthroughTransformer pipelines preserve explicitly supplied contexts, including empty mappings, during in-place mutations. Utterance and metadata transformations avoid self-merging. Transformer logging uses parameterized debug calls without filtering ChangesTransformer pipeline updates
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
ovos_plugin_manager/transformer_services.py (1)
258-260: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winMove session filtering behind the debug-level check.
The
safecomprehension copies every returned context, even when debug logging is disabled. This adds a top-level O(n) allocation to each transformer call. Create the filtered mapping only when the debug record will be emitted.Based on the PR objective to remove transformer runner hot-path overhead.
Also applies to: 297-299
🤖 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_plugin_manager/transformer_services.py` around lines 258 - 260, In the transformer logging paths around LOG.debug, guard construction of the filtered safe mapping with the debug-enabled check so the data comprehension runs only when the record will be emitted. Apply the same change to both referenced logging locations, while preserving removal of the session field and the existing debug message.
🤖 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.
Inline comments:
In `@ovos_plugin_manager/transformer_services.py`:
- Around line 267-268: Replace the truthiness-based context normalization with
an explicit None check in both methods containing the merge guard around
data/context, so a caller-provided empty dictionary remains the same object
passed to the plugin. Add a regression test that passes an empty context,
exercises an in-place plugin, and verifies the original object identity is
preserved.
---
Nitpick comments:
In `@ovos_plugin_manager/transformer_services.py`:
- Around line 258-260: In the transformer logging paths around LOG.debug, guard
construction of the filtered safe mapping with the debug-enabled check so the
data comprehension runs only when the record will be emitted. Apply the same
change to both referenced logging locations, while preserving removal of the
session field and the existing debug message.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c8014134-d4e9-4f85-b7eb-c26e592fa3f2
📒 Files selected for processing (2)
ovos_plugin_manager/transformer_services.pytest/unittests/test_transformer_services.py
Summary
merge_dictwhen a transformer returns a distinct context or deltaArchitecture and correctness
In-place transformers commonly return the exact context object they received. Merging that object into itself recursively walks identical nested mappings until
RecursionError. The transformer service owns context composition, so identity and explicit-Nonechecks belong here. No transformer is disabled, plugin order is unchanged, and distinct returned contexts still pass throughmerge_dict.The session no longer carries the sensitive credentials that motivated redaction. Logging now passes the full context directly with deferred
%sformatting, avoiding both the obsolete policy and an eager context copy. OpenVoiceOS/ovos-utils#415 supplies the shared disabled-level call-site gate.Validation
git diff --checkNo CI files were changed.