feat(retrospective): structured metrics + narrative contract with empty-period guard - #207
Merged
Merged
Conversation
…ty-period guard Rework GetRetrospectiveQuery to return computed metrics (completion rate, totals, active days, current/best streak, bad-habit slips, weekly consistency, top habits, needs-attention) plus a structured narrative, replacing the freeform LLM text so the client can render a stats dashboard. Add an empty-period guard: when a period has no completions and no bad-habit slips, fail with NoHabitsForPeriod instead of calling the LLM, so the UI shows a dedicated empty state rather than a hallucinated summary. - RetrospectiveMetricsCalculator computes metrics from habit logs - RetrospectiveNarrative domain model for the structured sections - AiRetrospectiveService builds the narrative; camelCase JSON serialization - Tests updated + added for the empty-period failure path Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
There was a problem hiding this comment.
This is a clean, well-scoped rework. The structured metrics + narrative split is the right design — deterministic numbers come from RetrospectiveMetricsCalculator and the LLM only touches prose, so neither can corrupt the other. A few things I verified:
- Cache key versioned to
retro:v2:— avoids serving the old cached string to the new client shape. - Logs are date-filtered at the EF Include level (line 68 of
GetRetrospectiveQuery.cs), soAccumulateWeekdayConsistencybuilding itscompletedDatesHashSet fromhabit.Logswithout a redundant date filter is correct. - Empty-period guard correctly gates on both completions AND bad-habit slips before invoking the LLM; the fallback path stores nothing in cache, which is right.
ParseNarrativefallback — partial-heading or unparseable AI output lands cleanly inHighlightswith empty siblings; the UI always gets a valid payload.streakState?.CurrentStreak ?? 0— null-safe for new users without streak history.- Test coverage — handler tests exercise the calculator end-to-end (static, no interface), covering top/needs-attention ordering, weekday consistency, bad-habit slip isolation, emoji passthrough, and the cache-hit/miss contract.
AiRetrospectiveServiceTestscovers the narrative parser with English, Portuguese, numbered headings, and fallback cases.
Authorization, validation, and CLAUDE.md rules all look good. Pairing this with orbit-ui-mobile #175 closes the contract loop.
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.



Backend for the retrospective rework, paired with the orbit-ui-mobile UX/QA batch.
What
Rework
GetRetrospectiveQueryto return computed metrics plus a structured narrative instead of freeform LLM text, so the client can render a stats dashboard.RetrospectiveMetricsCalculatorcomputes the metrics from habit logsRetrospectiveNarrativedomain model for the structured sectionsAiRetrospectiveServicebuilds the narrative; camelCase JSON serializationNoHabitsForPeriod(no LLM call) so the UI shows a dedicated empty state instead of a hallucinated summaryVerification
Paired PR
🤖 Generated with Claude Code