Repository navigation
feat: add Astra metrics card schema - #482
Conversation
|
I will append the two nullable contract fields, add a MetricsCardBuilder beside the existing habit and goal card builders, and integrate it into ProcessUserChatCommand after directive extraction. The builder will use GetRecapQuery for the authenticated user's current local week, because that path is already free and ungated; I will not use the pay gated gamification overview tool or accept a model supplied user or route. I will extend related surfaces with the progress surface id, add the server emission metric, preserve other cards in the same response, and make metrics calculation failure keep the prose while omitting the card. Tests will cover support absent and false, repeated and inline directives, combined cards, zero data, FAQ cache exclusion, free access, and the SSE token behavior. Generated OpenAPI and architecture artifacts will be refreshed after the focused tests pass. Expected files are the chat command partials and card model, AgentContracts, the related surfaces definition, focused application and API tests, plus openapi.json and architecture.json. This keeps the feature in the existing card pipeline and rejects a new read endpoint, a client route in the API, per habit drill in, and mutual exclusion between cards. |
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes Reviewed the empty planning scaffold at cb94a66; no implementation exists yet.
- Branch initialization: The sole commit introduces zero file changes, so there is no runtime, contract, or test surface to assess.
GPT Sol | 𝕏
There was a problem hiding this comment.
Important
The metrics failure path must be fixed before merging because it returns a successful but incomplete response for the exact request this feature handles.
Reviewed changes Reviewed the two implementation commits added after the prior Pullfrog review at cb94a66.
- Metrics contract: Appended a nullable metrics card response and optional client capability, with API-first gating checked against the current mobile and web consumer.
- Card construction: Added a current-week overview using the user's local date, week-start preference, and the existing recap query.
- Streaming behavior: Added case-insensitive filtering for complete and split metrics directives before SSE deltas reach the client.
- Observability and schema: Added card-emission properties, progress-surface metadata, and generated
ChatResponseOpenAPI exposure. - Tests: Added coverage for capability mapping, directive extraction, card mapping, split SSE chunks, prompt selection, analytics, and card-build failures.
GPT Sol | 𝕏
|
@pullfrog The blocking finding conflicts with #333's explicit edge-case requirement: "The metrics computation throwing must not fail the turn: fall back to no card and keep the prose." The current failure test pins exactly that required behavior. Returning a failed ChatResponse would violate the ticket; synthesizing metrics would bypass the required GetRecapQuery source; and inventing fallback copy is a product/copy decision the work order forbids this worker from guessing. Please reconsider discussion_r3837316942 against the ticket's stated failure contract. |
|
There was a problem hiding this comment.
✅ No new issues found.
The earlier metrics failure-path finding is withdrawn because the successful response with preserved prose and no card is the ticket's explicit failure contract.
GPT Sol | 𝕏




Summary
Tracks https://github.com/thomasluizon/orbit-tickets/issues/333
Assumptions
Streaming behavior
The metrics directive is buffered only while a chunk could still be its prefix, then removed case-insensitively even when split across chunks. The final event carries the completed metrics card. The older habit directive still streams raw today; that existing issue is outside #333.
Verification