fix(achievements): per-session badges no longer crash on set/secret definitions - #1
Merged
Merged
Conversation
…efinitions session_badges iterated every achievement definition and assumed each one could be evaluated against a single session. The engine's set-collection badges (set_autonomy, set_debugging, set_vibe, set_hermes_native, set_research, set_tools, set_models, set_lifestyle) and secret achievements carry no metric/threshold/requirements keys, so evaluate_boolean raised KeyError: 'metric' and the whole endpoint 500'd. 'Badges this session' therefore never populated for ANY session. Definitions that cannot be evaluated per-session are now skipped instead of crashing the response; the per-session view is correct to omit them (sets and secrets are account-level). Carries the enhanced achievements engine (full-history scan, evaluate_all, secret/collection definitions, rewards, quests, custom goals, XP levels, challenges) whose new definition kinds exposed the crash. Verified: plugin test suite 43/43; live session simulation returns 12 per-session badges.
…ed or include_inactive The scan previously crashed on SessionDB implementations (older Hermes cores, test fakes) whose list_sessions_rich lacks include_archived or whose get_messages lacks include_inactive. Both calls now fall back to the narrower signature on TypeError, so the engine keeps the archive and soft-deleted-message coverage on new backends and degrades gracefully on old ones. Repo integration suite (tests/plugins/test_achievements_plugin.py) 5/5 green; engine suite 43/43 green.
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
1 participant
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.
What broke
GET /api/plugins/hermes-achievements/sessions/{id}/badges("Badges this session") returned 500 Internal Server Error for every session.The endpoint loops every achievement definition and evaluates it against the session's aggregate. The engine's set-collection badges (
set_autonomy,set_debugging,set_vibe,set_hermes_native,set_research,set_tools,set_models,set_lifestyle) and secret achievements carry nometric/threshold_metric/requirementskeys.evaluate_definitionfalls through toevaluate_boolean, which doesdefinition["metric"]and raisesKeyError: 'metric'. One definition crashes the whole response, so the page always showed "No badges this session yet" even when the session had plenty of unlocks.The fix
session_badgesnow skips definitions that cannot be evaluated against a single session instead of crashing. That's the correct behavior for the per-session view: sets and secrets are account-level badges, not per-session ones. The response still returns every genuinely earned per-session badge.Verification
Scope note
This branch carries the enhanced achievements engine (full-history scan,
evaluate_all, secret/collection definitions, rewards, quests, custom goals, XP levels, challenges) whose new definition kinds exposed the crash. The committed engine predating it never had those definitions, so the fix and the engine ship together.Update: Added a compatibility commit (18aa959). The enhanced engine
calls
list_sessions_rich(include_archived=...)andget_messages(include_inactive=...), which the repo's integration suite(
tests/plugins/test_achievements_plugin.py) proved break on SessionDBvariants without those params. Both calls now fall back to the narrower
signature on
TypeError. Repo integration suite 5/5 green, engine suite43/43 green. The fix's headline behavior is unchanged: per-session badges
skip unevaluable definitions instead of 500ing.