feat(astra): structured habit-list card — Stage 2 backend#238
Conversation
|
Paired client render PR: thomasluizon/orbit-ui-mobile#300 |
There was a problem hiding this comment.
Solid implementation. The directive-token approach is a clean way to wire structured habit-list rendering without a protocol change: capable clients opt in via SupportsHabitListCard, get the prompt instruction, and receive the card; old clients are completely unaffected (the instruction is never injected and even if the token appeared it would be silently stripped). A few specifics that checked out:
- Production code correctly uses
context.UserToday(sourced fromIUserDateService.GetUserTodayAsync) everywhere, never rawDateTime.UtcNowfor user-facing dates. ResolveIncludedIdsfor the today scope correctly walks the ancestor chain so parent container habits appear alongside their due children, andResolveStatuscorrectly short-circuits onIsGeneralbefore date comparisons.TryExtractScopestrips the directive unconditionally (good defensive default) and only builds the card when capability is explicitlytrue.- All new code is covered: scope extraction, both card scopes, status variants, hierarchy/depth, ordering, and the three handler paths.
ChatResponseandAgentClientContextchanges are additive optional fields — no client breakage.
Stage 2 of the habit-listing fix: the real cure for mid-list drops. When a client signals it can render a habit-list card (clientContext supportsHabitListCard), Astra answers listing/schedule questions with a one-line intro plus a directive token instead of re-typing the habits. The handler builds an authoritative HabitListCard from the already-loaded habit context (scope today = due + overdue with ancestors; scope all = every active habit), strips the directive from the message, and attaches the card to ChatResponse - which already rides the final SSE event, so no protocol change. Backward compatible: the prompt instruction and the tool-free directive are gated on the client capability flag, so old clients keep today's text behavior (now with Stage 1's duplicate numbering). The card payload is intentionally light (id, title, emoji, depth, status) so the handler builds it from loaded entities without replaying the habits-list query. Clients render the card next (web + mobile, off this contract). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
09f9372 to
71677aa
Compare
There was a problem hiding this comment.
Re-approving after rebase onto main following #236 merge — diff is identical to what was reviewed, no new changes.
The implementation is clean: capability-gated directive token keeps old clients completely unaffected, ResolveIncludedIds correctly walks the ancestor chain for the today scope, ResolveStatus short-circuits on IsGeneral before date comparisons, and Habit.DueDate is non-nullable so those comparisons are safe. Tests cover all three handler paths (capable+directive, incapable+directive, capable+no-directive) and the full builder surface (scope extraction, both scopes, status variants, hierarchy/depth, ordering). No correctness, security, or contract issues found.
There was a problem hiding this comment.
Stage 2 of the habit-listing fix lands cleanly. The directive-token approach is the right abstraction: the model emits a one-line intro plus a structured token, the handler builds the authoritative card from already-loaded context data, and old clients are completely unaffected (no prompt injection, no directive, no card field). A few things that specifically held up under scrutiny: (1) TryExtractScope is called unconditionally — defensively correct, since stripping a stray token from a non-capable response is better than leaking it to the user, and the prompt instruction gate ensures the token shouldn't appear for non-capable clients anyway; (2) ResolveIncludedIds correctly excludes general habits from the today seed set while still bubble-walking their ancestors, and ResolveStatus short-circuits on IsGeneral before any DueDate comparison; (3) the habitList: null serialization for non-capable clients is consistent with the pre-existing pattern for all other nullable ChatResponse fields (PendingOperations, PolicyDenials, RelatedSurfaces), so no new consumer contract risk is introduced; (4) test coverage is thorough — both the card builder (scope, status, hierarchy, ordering) and the three handler paths are exercised.
There was a problem hiding this comment.
The habit-list card feature lands cleanly with no correctness, security, or contract issues. The directive-token approach is properly gated on SupportsHabitListCard so old clients are completely unaffected — no prompt injection, no directive, no HabitList field. ResolveIncludedIds correctly walks the ancestor chain for the today scope while excluding general habits from the seed set, and ResolveStatus short-circuits on IsGeneral before any DueDate comparison. ChatResponse and AgentClientContext changes are purely additive optional fields consistent with the pre-existing pattern. Test coverage is thorough across all three handler paths and the full builder surface (scope extraction, both scopes, status variants, hierarchy/depth, ordering), confirmed by SonarCloud at 97.7% new-code coverage.
There was a problem hiding this comment.
Stage 2 of the habit-listing fix is solid. The directive-token approach is the right abstraction for this problem: the model emits a one-line intro plus a structured token, the handler builds the authoritative card from already-loaded context data, and old clients are completely unaffected (no prompt injection, no directive, no card field). Key things that checked out: (1) TryExtractScope strips the directive unconditionally — correct defensive behavior that prevents the raw token leaking to non-capable clients, while the prompt-injection gate ensures it should only appear for capable clients anyway; (2) ResolveIncludedIds correctly seeds the today set from non-general habits with DueDate ≤ today, then bubble-walks ancestors unconditionally to preserve the hierarchy for tree rendering — ResolveStatus correctly labels general ancestors StatusGeneral; (3) ChatResponse and AgentClientContext changes are purely additive nullable fields appended last, so all positional callers remain safe; (4) test coverage is thorough — scope extraction, both card scopes, status variants, ancestor/depth propagation, ordering, and all three handler paths.
|
Mirror the #238 habit-list card for goals: Astra emits a [[orbit:goals]] directive when asked to show/list/review goals; the handler detects it, builds a GoalListCard (active goals with progress + deadline) for clients advertising SupportsGoalListCard, and returns it in ChatResponse. Old clients (no capability flag) keep the prose fallback — append-only contract, no breaking change. Refs thomasluizon/orbit-ui-mobile#301 Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>



Stage 2 of the Astra habit-listing fix — the real cure for mid-list drops
Stage 1 (#236) made duplicate siblings un-collapsible and guarded truncation, but the model still relays the whole list as free text, so middle items can still drop (lost-in-the-middle). Stage 2 stops the relay: the app renders the list, the model just writes a one-line intro.
How it works (capability-gated, additive)
clientContext.supportsHabitListCard: true(new optional field onAgentClientContext).[[orbit:habits:today]]/[[orbit:habits:all]]and not enumerate.HabitListCardfrom the already-loaded habit context (today= due + overdue with ancestors;all= every active habit), strips the directive from the message, and attaches the card toChatResponse.HabitList.ChatResponsealready rides the final SSE event, so no streaming-protocol change — the card flows through buffered and streaming paths alike.Chose a directive token over a new
render_habit_listtool deliberately: every tool here is wired through the full agent capability/operation/policy executor, which is hugely disproportionate for a presentation signal. The directive touches only additive surfaces and the handler builds the payload from data it already has (noToolResult.Payloadescape, no new capability).Backward compatibility
Fully additive. Old clients never send the flag → no prompt instruction, no directive → they keep today's text behavior (with Stage 1's duplicate numbering). The unknown
clientContextfield is ignored by older API builds too, so deploy order is safe in both directions (API-first is still preferred).Tests
HabitListCardBuilder(directive parse + scope/hierarchy/status), handler wiring (capable populates + strips; non-capable strips only; no-directive omits). Full suite green: 3564 passed.Cross-repo
Paired with the client render PR in
orbit-ui-mobile(link added below). Stacked on #236 — base isfix/astra-habit-listing; will retarget tomainafter #236 merges.🤖 Generated with Claude Code