refactor(gateway): share error-text blob between not_found classifiers - #248
Merged
Conversation
|
Review Complete Risk: 🟢 Low (12/100) — no findings · 90 LOC across 3 files Minor refactoring in the gateway delivery pipeline: argument ordering change in BasePlatformAdapter and corresponding callers, with a new is_chat_level_not_found helper on DeliveryTarget. No bugs or regressions detected. Files Reviewed (3 files) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to NousResearch#55780 / NousResearch#56225 (dead-target
not_foundblast-radius fix, merged).Why
classify_send_errorandis_chat_level_not_foundeach build a lowercased error blob to substring-match against — but divergently:classify_send_errorappends the exception class name (exc.__class__.__name__), whileis_chat_level_not_foundappends onlystr(exc). A caller passingexc=to both could get inconsistent answers on the same failure. The two functions' signatures also had swapped positional order ((exc, error_text)vs(error_text, exc)), an easy footgun.Surfaced by the hermes-pr-review Phase 2c structured review of NousResearch#56225; the merge landed the base fix before this follow-up could ride along, so it's a standalone follow-up.
Changes
_error_blob(exc, error_text)as the single source of truth both classifiers use (includesstr(exc)when non-empty + the class name; skips an emptystr(exc)so no stray leading space).is_chat_level_not_foundto(exc, error_text), matchingclassify_send_error; update the sole caller (gateway/delivery.py) and the three tests to keyword form._error_blobkeeps the class name (mutation-verified: removing the class-name append fails the test).Tests
No behavior change to the classifier's current call sites (they pass
error_text=only).Mirror-of: NousResearch#56241
NousResearch#56241