feat: ClarificationCard for NeedsClarification chat responses - #100
Merged
Conversation
Renders structured clarification card with quick-action buttons when the AI needs to ask about an ambiguous habit schedule. Tapping a button resolves the clarification server-side, invalidates habit queries, and shows local success state. - Shared: extends actionStatusSchema with NeedsClarification, adds clarificationRequestSchema and quickActionSchema, new endpoint constant, habits.clarification.* i18n keys (en + pt-BR) - Web: ClarificationCard + useResolveClarification hook + resolveClarification server action + message-bubble dispatch - Mobile: parity-paired ClarificationCard (RN) + hook (apiClient) + message-bubble dispatch - Tests: 5 web + 5 mobile cases Closes #98 Closes #99 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds NeedsClarification contracts and i18n; a clarification-resolve API helper and web server action; client hooks; ClarificationCard components for web and mobile with message-bubble wiring; and unit tests for resolve success, expired (404), and generic error cases. ChangesChat Clarification Flow
Sequence DiagramsequenceDiagram
participant User
participant ChatUI as Chat UI
participant ClientHook as ResolveAction
participant API as ClarifyAPI
User->>ChatUI: Views ClarificationCard and selects an option
ChatUI->>ClientHook: Call resolveClarification(operationId, value)
ClientHook->>API: POST value
API-->>ClientHook: Response with operation.status
ClientHook-->>ChatUI: Return result
ChatUI->>ChatUI: Render success or error and invalidate on success
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
This was referenced May 19, 2026
Merged
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 885c68cf4a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/shared/src/types/chat.ts (1)
116-126:⚠️ Potential issue | 🟠 Major | ⚡ Quick winEnforce
clarificationRequestwhen status isNeedsClarification.
actionResultSchemacurrently allowsstatus: 'NeedsClarification'with noclarificationRequest, which can cause the clarification flow to vanish at runtime.Suggested fix
export const actionResultSchema = z.object({ type: aiActionTypeSchema, status: actionStatusSchema, entityId: z.string().nullable(), entityName: z.string().nullable(), error: z.string().nullable(), field: z.string().nullable(), suggestedSubHabits: z.array(suggestedSubHabitSchema).nullable(), conflictWarning: conflictWarningSchema.nullable(), clarificationRequest: clarificationRequestSchema.nullable().optional(), -}) +}).superRefine((value, ctx) => { + if (value.status === 'NeedsClarification' && !value.clarificationRequest) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: ['clarificationRequest'], + message: 'clarificationRequest is required when status is NeedsClarification', + }) + } +})🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/shared/src/types/chat.ts` around lines 116 - 126, The actionResultSchema allows status to be NeedsClarification without a clarificationRequest, causing lost clarification flows; update actionResultSchema (the z.object with fields type, status, clarificationRequest, etc.) to add a Zod refinement/superRefine that checks when status === 'NeedsClarification' then clarificationRequest must be non-null/non-empty and otherwise allow nullable, and return a clear path-specific error (e.g., on 'clarificationRequest') if missing; ensure the check uses the same actionStatusSchema enum value (NeedsClarification) so validation enforces the presence of clarificationRequest when required.
🧹 Nitpick comments (1)
.agents/plans/completed/chat-clarification.plan.md (1)
232-246: ⚡ Quick winMark superseded tasks in this completed plan to avoid implementation drift confusion.
This completed plan still reads as if
ResolveClarificationCommandand related tests/integration tests were implemented, while the report records a controller-inline deviation. Add an explicit “superseded by implementation deviation” note beside these tasks (or link directly to the deviation section) so future readers don’t treat these as delivered artifacts.Also applies to: 393-405, 447-464
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.agents/plans/completed/chat-clarification.plan.md around lines 232 - 246, Update the completed plan to mark tasks that were not implemented as originally specified as "superseded by implementation deviation": add a clear superseded note next to the entries for ResolveClarificationCommand (src/Orbit.Application/Chat/Commands/ResolveClarificationCommand.cs), the AiController change (src/Orbit.Api/Controllers/AiController.cs), the PendingClarification store/DB artifacts, and the associated test files (ResolveClarificationCommandTests, ClarificationFlowTests, CreateHabitToolClarificationTests) indicating they were replaced by a controller-inline implementation; either add a one-line “Superseded by implementation deviation — see deviation section” next to each affected bullet or link directly to the deviation section so future readers don’t treat these as delivered artifacts.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.agents/reports/chat-clarification-report.md:
- Line 128: The report table row referencing the plan file uses the wrong path
for chat-clarification.plan.md; update the table entry in
.agents/reports/chat-clarification-report.md so the plan link points to the
committed location under the plans completed subdirectory (i.e., change the
target to the completed plan location for chat-clarification.plan.md) to restore
correct traceability.
In `@apps/mobile/__tests__/components/chat/clarification-card.test.tsx`:
- Around line 4-5: The test imports and uses TestRenderer (TestRenderer.create
and TestRenderer.act) which violates the repo guideline to use React Testing
Library; replace TestRenderer usage with React Testing Library utilities: import
{ render, screen, act } from '`@testing-library/react`' (or from test-utils if you
have a wrapper), change TestRenderer.create(<ClarificationCard ... />) to
render(<ClarificationCard clarificationRequest={baseClarification} />), replace
assertions that rely on the TestRenderer output with DOM queries via screen
(e.g., getByText/getByRole), and convert TestRenderer.act(...) blocks to the
testing-library act or the async utilities (await act(async () => ...) or use
waitFor) around actions that cause updates so all occurrences of TestRenderer,
TestRenderer.create, TestRenderer.act, and usage with
ClarificationCard/baseClarification are migrated accordingly.
- Around line 6-14: Replace all uses of the any type with unknown and add proper
narrowing/type guards or concrete typings: change colorProxy's declaration from
any to a typed Proxy (e.g., Proxy<Record<string, string> | unknown, ...> using
unknown where appropriate and narrowing to string when accessing values), update
mocked component props (replace any with the component prop interface or unknown
then cast after a guard), change helper function parameters from any to unknown
and add runtime type checks before use, and update TestRenderer tree variables
and node-tree callback parameters from any to unknown and narrow them with type
predicates or explicit casts where their shape is asserted; reference symbols to
adjust: colorProxy, the mocked prop variables passed to the component render
calls, the helper functions used in tests, and the TestRenderer/tree node
callback functions so that all previous any annotations are replaced with
unknown and safely narrowed before use.
In `@apps/web/components/chat/clarification-card.tsx`:
- Around line 70-77: clarificationRequest.question and action.label are being
cast to IntlKey and passed directly to t() (in the JSX around
clarificationRequest.question and inside the quickActions map) which breaks when
backend provides literal text; update the rendering to use the same defensive
fallback as mobile: call t(clarificationRequest.question as IntlKey, {
defaultValue: clarificationRequest.question }) and for each action use
t(action.label as IntlKey, { defaultValue: action.label }), keeping the existing
activeValue logic intact so literal strings render when no translation key
exists.
---
Outside diff comments:
In `@packages/shared/src/types/chat.ts`:
- Around line 116-126: The actionResultSchema allows status to be
NeedsClarification without a clarificationRequest, causing lost clarification
flows; update actionResultSchema (the z.object with fields type, status,
clarificationRequest, etc.) to add a Zod refinement/superRefine that checks when
status === 'NeedsClarification' then clarificationRequest must be
non-null/non-empty and otherwise allow nullable, and return a clear
path-specific error (e.g., on 'clarificationRequest') if missing; ensure the
check uses the same actionStatusSchema enum value (NeedsClarification) so
validation enforces the presence of clarificationRequest when required.
---
Nitpick comments:
In @.agents/plans/completed/chat-clarification.plan.md:
- Around line 232-246: Update the completed plan to mark tasks that were not
implemented as originally specified as "superseded by implementation deviation":
add a clear superseded note next to the entries for ResolveClarificationCommand
(src/Orbit.Application/Chat/Commands/ResolveClarificationCommand.cs), the
AiController change (src/Orbit.Api/Controllers/AiController.cs), the
PendingClarification store/DB artifacts, and the associated test files
(ResolveClarificationCommandTests, ClarificationFlowTests,
CreateHabitToolClarificationTests) indicating they were replaced by a
controller-inline implementation; either add a one-line “Superseded by
implementation deviation — see deviation section” next to each affected bullet
or link directly to the deviation section so future readers don’t treat these as
delivered artifacts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4eea4d25-2468-4612-83d4-456020ac5b7d
📒 Files selected for processing (15)
.agents/plans/completed/chat-clarification.plan.md.agents/reports/chat-clarification-report.mdapps/mobile/__tests__/components/chat/clarification-card.test.tsxapps/mobile/components/chat/clarification-card.tsxapps/mobile/components/message-bubble.tsxapps/mobile/hooks/use-resolve-clarification.tsapps/web/__tests__/components/chat/clarification-card.test.tsxapps/web/app/actions/chat.tsapps/web/components/chat/clarification-card.tsxapps/web/components/chat/message-bubble.tsxapps/web/hooks/use-resolve-clarification.tspackages/shared/src/api/endpoints.tspackages/shared/src/i18n/en.jsonpackages/shared/src/i18n/pt-BR.jsonpackages/shared/src/types/chat.ts
Review summaryGood cross-platform coverage and clean architecture overall. One correctness bug and a few policy violations need addressing before merge. Must fix
Should fix
Looks good
🤖 Generated with Claude Code |
Frontend hardening from review:
- Cards (web + mobile): gate success state on operation.status ===
'Succeeded'. HTTP 200 with status Denied/Failed/PendingConfirmation
no longer renders a misleading green "Created" message — shows the
generic error instead.
- Hooks (web + mobile): switch invalidation from onSettled to onSuccess
and additionally gate on operation.status === 'Succeeded'. Cache no
longer wastefully invalidates on 4xx/5xx or denied operations.
- Shared schema: actionResultSchema gets a superRefine that enforces
clarificationRequest is present when status is NeedsClarification.
Removed the unused clarificationResolveResponseSchema and
ClarificationResolveResponse type (dead code; the action and hook
correctly use AgentExecuteOperationResponse which matches the
backend's actual response shape).
- message-bubble (web + mobile): narrowed the filter type predicate so
action.clarificationRequest is non-null by type, dropping the !
non-null assertions.
- Web card: replaced unsafe `as IntlKey` casts with a translateOrLiteral
helper that falls back to the literal when next-intl can't find the
key (matches mobile's defaultValue pattern).
- Mobile card: added defaultValue fallback to the action.label
translation call (it was already on the question).
- Mobile test: eliminated all 12 `any` annotations per AGENTS.md ("Zero
any. Use unknown with narrowing."). Local TestNode/TestInstance
interfaces describe the react-test-renderer shape.
- Tests: added "non-Succeeded operation" cases on web + mobile that
prove the false-success bug is closed.
- Report: fixed the plan path to point to the archived location.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Code Review — PR #100Recommendation: APPROVE with minor fixes SummarySolid implementation. Architecture is clean, cross-platform parity is complete, i18n keys landed in both locales, and the Issues (inline comments posted)
Note: the PR description mentions Safe to merge after the UUID validation gap is addressed. The dead condition and unused i18n key can follow in a cleanup PR. |
- resolveClarification server action now rejects non-UUID operationId before interpolating into the request URL. Server Actions can be invoked directly without going through the React component, so the guard is at the action level rather than only the component. - i18n: removed the unused habits.clarification.submitting key from en.json and pt-BR.json — never wired up; the loading state is conveyed by the spinner icon alone. - Web ClarificationCard: dropped the translateOrLiteral helper. Its dot-containing-key branch returned the same string as the fallback, so the condition was dead. next-intl returns the key verbatim on a miss, which naturally doubles as the literal fallback path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Review summaryOverall this is solid work — parity is complete, i18n is correct in both locales, no Two issues flagged inline:
Minor note: The PR description lists |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/components/chat/clarification-card.tsx`:
- Line 6: The import of the ClarificationRequest type is using the deeper barrel
'`@orbit/shared/types/chat`' instead of the shared types barrel; change the import
for ClarificationRequest to come from '`@orbit/shared/types`' (update the import
statement that currently references ClarificationRequest) so it follows the
repository contract for shared type imports.
- Line 95: Remove the disallowed color transition from the button class on the
ClarificationCard UI: in the element whose className contains "inline-flex
items-center ... transition-colors duration-150 ..." remove "transition-colors"
(or replace it with "transition-opacity" or "transition-transform" if you want a
permitted animation) so only transform/opacity transitions are used; keep the
rest of the className unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2bee8089-63d8-42a9-964a-9091eb80b7f4
📒 Files selected for processing (4)
apps/web/app/actions/chat.tsapps/web/components/chat/clarification-card.tsxpackages/shared/src/i18n/en.jsonpackages/shared/src/i18n/pt-BR.json
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/shared/src/i18n/en.json
- apps/web/app/actions/chat.ts
- resolveClarification server action: also bounds-check `value` (string, non-empty, <= 2048 chars) — mirrors AppConstants on the backend. operationId was already UUID-validated; this gives both inputs symmetric client guards for direct-Server-Action callers. - Web ClarificationCard: wrap handleSelect's mutateAsync in try/catch/finally so that unmount-mid-await, query-abort, or any future throw can't leave the tapped button permanently disabled. Matches the pattern already used in the mobile card. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Review summaryOverall this is well-executed — cross-platform parity is solid, i18n is complete for both locales, no Issues flagged (4 inline comments)
Minor note on PR description: the mobile test file has 7 test cases, not 5 as stated; more coverage is good, just update the count. Item 1 is the only blocker — the rest are suggestions. |
… error - Web + mobile cards: map status 409 (Conflict — already resolved) to a new habits.clarification.errorAlreadyResolved key instead of falling through to the generic error. en + pt-BR locale entries added. 404 → errorExpired stays as before. - Extracted both card status → error-key mappings into a small mapStatusToErrorKey helper so the table is a single line. - Web quick-action button: added focus-visible:ring styling so keyboard navigation surfaces a visible focus state per CLAUDE.md "every clickable element needs hover, focus-visible, and active". - apps/mobile/components/chat/clarification-card.tsx and the test file: ClarificationRequest now imported from @orbit/shared/types (barrel) instead of the deep types/chat path, matching AGENTS.md. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Review: feat/ClarificationCard — NeedsClarification flow Recommendation: NEEDS WORK (minor) What is good:
Issues: [High] 410 vs 404 for TTL expiry — inline suggestions on both clarification-card files Plan D7 states the backend returns 410 Gone for TTL expiry, but mapStatusToErrorKey maps only 404 to errorExpired; 410 falls through to errorGeneric. If the backend follows the plan, expired clarifications show "Something went wrong" instead of "This clarification expired". Please confirm which status code the API actually returns for an expired operation. [Medium] Missing 409 test in both platforms — inline suggestions on both test files mapStatusToErrorKey handles 409 to errorAlreadyResolved but no test exercises this path in either platform's test file. Generated with Claude Code |
- Both cards' mapStatusToErrorKey now treats 404 and 410 identically (errorExpired). Backend currently returns 404 for a vanished row; 410 is the semantically-correct HTTP status for an expired resource and would be safe to switch to without a frontend change. Forward- compatible. - Added two new web Vitest cases and two new mobile cases covering the 409 (already-resolved) and 410 (gone) → error-key mappings, so the table is exercised end-to-end. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Code Review — PR #100Recommendation: APPROVE with minor fixes Solid implementation of the Issues
Notes
|
- Web + mobile error texts now act as ARIA live regions: role=\"alert\" on the web <p>, accessibilityLiveRegion=\"polite\" + accessibilityRole=\"alert\" on the mobile <Text>. Assistive tech announces resolve failures when they appear. - Web quick-action button: added transition-transform duration-150 so active:scale-[0.98] eases instead of snapping. Still complies with the transform/opacity-only animation rule. - quickActionSchema.value: .min(1) so an empty value never round- trips through the client — the backend already rejects but the schema is now honest about it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Review: feat/ClarificationCard — APPROVESolid implementation. Full web/mobile parity maintained, all user-facing strings through i18n (both locales), zero Issues: 0 critical · 0 high · 0 medium · 3 low (inline comments posted)
Validation
Generated with Claude Code |
- Both ClarificationCards: comment noting that PendingConfirmation isn't expected on the resolve path (clarifications use HabitsWrite, no step-up). Falls through to errorGeneric and the user can re- initiate from chat — acceptable MVP trade-off, now documented. - chat.ts `clarificationRequest`: kept `.nullable().optional()` but added a comment explaining the dual modifier — nullable covers the wire format (System.Text.Json writes null), optional is the cheap belt for existing fixtures that don't set the field. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
| question: z.string(), | ||
| operationId: z.string().uuid(), | ||
| missingArgumentKey: z.string(), | ||
| quickActions: z.array(quickActionSchema), |
There was a problem hiding this comment.
Medium — defensive schema gap
quickActions has no minimum length constraint. If the backend ever sends an empty array (or a bug causes it), the card renders an unanswerable question — the user sees the prompt but has no buttons to respond.
Suggested change
| quickActions: z.array(quickActionSchema), | |
| quickActions: z.array(quickActionSchema).min(1), |
This also makes the intent explicit: a ClarificationRequest with zero options is malformed by definition.
|
| ) | ||
| } | ||
|
|
||
| function mapStatusToErrorKey(status: number): string { |
There was a problem hiding this comment.
Suggestion — duplicated helper
mapStatusToErrorKey is byte-for-byte identical to the one in apps/mobile/components/chat/clarification-card.tsx:128. Since both platforms share the same HTTP status codes and error key names, this could live in @orbit/shared/utils and be imported on both sides, keeping the mapping in one place if status codes ever change.
Plan: Chat Clarification — Stop silent one-time tasks + structural
|
| Field | Value |
|---|---|
| Type | BUG_FIX + NEW_CAPABILITY |
| Complexity | HIGH |
| Repos | both |
| Parity Required | yes |
| GitHub Issues | #98, #99 |
| Web Affected | yes |
| Mobile Affected | yes |
| API Affected | yes |
Key design decisions
D1 — Resume mechanism: server-side store
Picked Option 2 from #99's tech notes. New PendingClarification entity + PendingClarificationStore + POST /api/ai/clarifications/{operationId}/resolve.
Why:
- Survives page reload (state doesn't live in JS component state).
- Avoids LLM re-interpretation: the resolved arg is merged deterministically by the backend, not re-parsed by the model.
- Codebase already has a precedent (
PendingAgentOperationStoreatsrc/Orbit.Infrastructure/Services/PendingAgentOperationStore.cs). Frontend pattern already exists (pending-operation-card.tsxfollows the same callback-driven shape). - The brittle multilingual
[clarification:frequency=daily]synthetic-message approach is rejected.
Why a NEW store rather than overloading PendingAgentOperationStore:
- Confirmation operations have a policy / fingerprint / SHA256-token / step-up lifecycle.
- Clarifications have none of that — they're a partial-args stash with a missing field to fill in.
- Overloading the table would force conceptual mixing.
D2 — CreateHabitTool heuristic
The tool returns NeedsClarification iff both:
frequency_unitis absent (parsed asnull)titlecontains case-insensitive substring"habit","rotina", or"hábito"(matches both English and pt-BR)
Intentionally narrow. Verb-phrase detection is unreliable in C# without NLP. Broader cases are handled by the prompt rule from #98, which fires before the tool is called.
D3 — Status sits on ActionResult, payload on a new field
Mirror the existing Suggestion pattern: ActionResult.Status = NeedsClarification, and add ActionResult.ClarificationRequest: ClarificationRequest? next to SuggestedSubHabits.
In the handler at ProcessUserChatCommand.cs:413 (special-case for suggest_breakdown), add a parallel branch that recognizes a ClarificationRequest payload from ToolResult.Payload and packages it into ActionStatus.NeedsClarification. Before returning, the handler calls PendingClarificationStore.Create(...) to stash the partial args server-side.
D4 — Capability registration
Add AgentCapabilityIds.ClarifyHabitArguments (or reuse HabitsWrite?). Pick: reuse HabitsWrite. Clarification is a synthetic intermediate state of create_habit — same capability, same risk class. No new policy gate needed.
D5 — Resolve endpoint authorization
POST /api/ai/clarifications/{operationId}/resolve is authenticated (JWT bearer) and scoped to the owning user. Store lookup compares operationId.UserId == principal.UserId and rejects 404 otherwise. No fresh-confirmation token needed — clarifications are non-destructive intermediate steps.
D6 — Re-execution
On resolve, the endpoint:
- Loads the stashed
PartialArgumentsJson+MissingArgumentKey. - Merges the user's
valueinto the partial args (e.g.,frequency_unit = "Day",frequency_quantity = 1). - Constructs an
AgentExecuteOperationRequestfor the original tool (create_habit). - Dispatches via
IAgentOperationExecutor.ExecuteAsync(same path the chat handler uses). - Marks the clarification resolved (one-shot — second call returns 410 Gone).
- Returns a minimal
ClarificationResolveResponse { status, entityId?, entityName?, error? }.
The frontend, on success, triggers queryClient.invalidateQueries({ queryKey: habitKeys.lists() }) and renders a local success state in the card — mirroring BreakdownSuggestion.onConfirmed.
D7 — Lifecycle / TTL
PendingClarification rows expire after 30 minutes (matching AgentPlatformSettings.PendingOperationTtlMinutes). After expiry the resolve endpoint returns 410 Gone; the frontend renders "this clarification expired — please ask again."
Patterns to follow
Backend — Tool returning a structured payload
Existing precedent: SuggestBreakdownTool returns ToolResult(success: true, EntityName: title) and the handler at ProcessUserChatCommand.cs:413-420 special-cases the name to surface SuggestedSubHabits.
// SOURCE: src/Orbit.Application/Chat/Commands/ProcessUserChatCommand.cs:413-420
if (call.Name == "suggest_breakdown")
{
return new ActionResult(
ToolNameToPascalCase(call.Name),
ActionStatus.Suggestion,
EntityName: result.EntityName,
SuggestedSubHabits: ExtractSuggestedSubHabits(call.Args));
}We will not add another name-keyed special case. Instead, the tool returns the payload on ToolResult.Payload and the handler checks if (result.Payload is ClarificationRequest cr) { ... }. This generalizes the pattern so future tools (delete-by-name disambiguation, bulk-op scope) can adopt NeedsClarification without editing the handler.
Backend — Pending store (precedent to mirror, not extend)
// SOURCE: src/Orbit.Infrastructure/Services/PendingAgentOperationStore.cs
public async Task<PendingAgentOperation> Create(/* ... */) {
// checks for existing, builds entity, saves to DB, returns view
}New PendingClarificationStore mirrors the shape but drops everything related to confirmation tokens, step-up, fingerprint dedup, policy state. Just: create → get → resolve (one-shot).
Backend — Endpoint precedent
// SOURCE: src/Orbit.Api/Controllers/AiController.cs:254-296
[HttpPost("pending-operations/{id:guid}/execute")]
public async Task<IActionResult> Execute(Guid id, [FromBody] ExecutePendingOperationRequest body, CancellationToken ct) { ... }New endpoint sits as a sibling: [HttpPost("clarifications/{operationId:guid}/resolve")] on the same controller.
Backend — Prompt section
// SOURCE: src/Orbit.Infrastructure/Services/Prompts/Sections/Static/StructuringStrategySection.cs
public int Order => 250;
public bool ShouldInclude(PromptContext context) => true;
public string Build(PromptContext context) { /* return rule list */ }Add a NEW section ClarificationGuidanceSection (e.g. Order = 260, always include) that teaches the model: "You may return NeedsClarification from any tool by setting clarification on the response. The user will see a card with your question + quick actions. Prefer this over plain text when the missing argument is a discrete enum (frequency unit, day of week, scope)."
Frontend — Status dispatch (web)
// SOURCE: apps/web/components/chat/message-bubble.tsx (lines ~53-138)
// Existing filters Suggestion and routes to <BreakdownSuggestion>
const suggestionActions = actions.filter(a => a.status === 'Suggestion' && a.suggestedSubHabits?.length)
const nonSuggestionActions = actions.filter(a => a.status !== 'Suggestion' /* not Suggestion */)Add a parallel filter for a.status === 'NeedsClarification' && a.clarificationRequest. Mount <ClarificationCard> instead of <BreakdownSuggestion> for those.
Frontend — Mobile dispatch
// SOURCE: apps/mobile/components/message-bubble.tsx (lines ~115-234)Same structure, React Native primitives.
Frontend — Card visual conventions
Match BreakdownSuggestion exactly:
- Web:
bg-surface-elevated/50 border border-border-muted rounded-[var(--radius-xl)] p-4 shadow-[var(--shadow-sm)] - Mobile:
backgroundColor: colors.surfaceOverlay, borderRadius: radius.xl, padding: 16, ...shadows.sm, elevation: 2
Quick-action buttons follow the existing chip pattern at apps/mobile/components/chat/suggestion-chips.tsx:54-61 (full-pill borderRadius: 9999, bg-surface-elevated, tap → submit). Web uses Tailwind equivalents.
Frontend — Hook precedent
// SOURCE: apps/web/hooks/use-habits.ts (TanStack mutation pattern)
export function useBulkCreateHabits() {
return useMutation({ mutationFn: ..., onSuccess: () => queryClient.invalidateQueries({ queryKey: habitKeys.lists() }) })
}Create useResolveClarification in both apps/web/hooks/ and apps/mobile/hooks/ using the same mutation shape.
Shared — Endpoint constant precedent
// SOURCE: packages/shared/src/api/endpoints.ts
export const API = {
// ... existing nested structure ...
}Add ai.clarifications.resolve(operationId) next to the existing pending-operation endpoints.
Shared — Zod chat types
// SOURCE: packages/shared/src/types/chat.ts:34-36
export const actionStatusSchema = z.enum(['Success', 'Failed', 'Suggestion'])Becomes z.enum(['Success', 'Failed', 'Suggestion', 'NeedsClarification']). Add clarificationRequestSchema (zod) with the matching shape from the backend record.
Tests
// SOURCE: tests/Orbit.Application.Tests/Chat/Tools/BulkLogHabitsToolTests.cs
public class BulkLogHabitsToolTests {
[Fact] public async Task ExecutesSuccessfully() { ... }
}// SOURCE: apps/web/__tests__/components/chat/breakdown-suggestion.test.tsx
test('renders + cancels + submits', () => { ... })Files to change
orbit-api (branch fix/chat-clarification)
| File | Action | Purpose |
|---|---|---|
src/Orbit.Application/Chat/Tools/Implementations/CreateHabitTool.cs |
UPDATE | Tighten Description (#98); add heuristic that returns ToolResult with ClarificationRequest payload (#99) |
src/Orbit.Infrastructure/Services/Prompts/Sections/Static/StructuringStrategySection.cs |
UPDATE | Add rule between line 49 and 51: "If the user calls something a habit/rotina/hábito without stating a schedule, ASK before calling create_habit." (#98) |
src/Orbit.Application/Chat/Commands/ProcessUserChatCommand.cs |
UPDATE | Add NeedsClarification to ActionStatus enum (line 47); add ClarificationRequest? ClarificationRequest to ActionResult (line 38-45); add handler branch to recognize ToolResult.Payload is ClarificationRequest and stash it via IPendingClarificationStore before returning |
src/Orbit.Application/Chat/Models/ClarificationRequest.cs |
CREATE | Record: Question, OperationId, MissingArgumentKey, QuickActions: IReadOnlyList<QuickAction> |
src/Orbit.Application/Chat/Models/QuickAction.cs |
CREATE | Record: Label, Value, Description? |
src/Orbit.Application/Chat/Commands/ResolveClarificationCommand.cs |
CREATE | MediatR command + handler. Loads pending clarification, merges value, dispatches via IAgentOperationExecutor, returns ClarificationResolveResponse |
src/Orbit.Application/Common/Interfaces/IPendingClarificationStore.cs |
CREATE | Methods: Create, GetById, Resolve |
src/Orbit.Domain/Entities/PendingClarification.cs |
CREATE | Id, UserId, ToolName, PartialArgumentsJson, MissingArgumentKey, Question, QuickActionsJson, CreatedAtUtc, ResolvedAtUtc?, ExpiresAtUtc |
src/Orbit.Infrastructure/Services/PendingClarificationStore.cs |
CREATE | DB-backed implementation |
src/Orbit.Infrastructure/Persistence/OrbitDbContext.cs |
UPDATE | Add DbSet<PendingClarification> PendingClarifications |
src/Orbit.Infrastructure/Persistence/Configurations/PendingClarificationConfiguration.cs |
CREATE | EF configuration (PK, indexes on UserId + CreatedAtUtc, ExpiresAtUtc) |
src/Orbit.Infrastructure/Migrations/<ts>_AddPendingClarifications.cs |
CREATE | EF migration |
src/Orbit.Api/Controllers/AiController.cs |
UPDATE | Add POST /api/ai/clarifications/{operationId:guid}/resolve — dispatches ResolveClarificationCommand |
src/Orbit.Infrastructure/Services/Prompts/Sections/Static/ClarificationGuidanceSection.cs |
CREATE | New prompt section explaining the NeedsClarification flow to the model |
src/Orbit.Infrastructure/Services/SystemPromptBuilder.cs |
UPDATE | Register ClarificationGuidanceSection in _sections |
src/Orbit.Api/Extensions/ServiceCollectionExtensions.cs |
UPDATE | DI: AddScoped<IPendingClarificationStore, PendingClarificationStore>() |
tests/Orbit.Application.Tests/Chat/Tools/CreateHabitToolClarificationTests.cs |
CREATE | Unit tests: triggers (habit keyword + no freq), bypasses (one-time markers, freq present, non-habit title) |
tests/Orbit.Application.Tests/Chat/Commands/ResolveClarificationCommandTests.cs |
CREATE | Unit tests: happy path, expired, already-resolved, wrong-user, missing arg key |
tests/Orbit.IntegrationTests/Chat/ClarificationFlowTests.cs |
CREATE | E2E: chat returns NeedsClarification → resolve endpoint → habit created with correct frequency |
orbit-ui-mobile (branch fix/chat-clarification)
| File | Action | Purpose |
|---|---|---|
packages/shared/src/types/chat.ts |
UPDATE | Add 'NeedsClarification' to actionStatusSchema; add clarificationRequestSchema + quickActionSchema; add clarificationRequest field to actionResultSchema |
packages/shared/src/api/endpoints.ts |
UPDATE | Add ai.clarifications.resolve(operationId) |
packages/shared/src/query/keys.ts |
UPDATE | (optional) no new keys needed — re-uses habitKeys.lists() for invalidation |
packages/shared/src/i18n/en.json |
UPDATE | Add habits.clarification.* keys (see below) |
packages/shared/src/i18n/pt-BR.json |
UPDATE | pt-BR equivalents |
apps/web/components/chat/clarification-card.tsx |
CREATE | Web <ClarificationCard> — matches BreakdownSuggestion visual language |
apps/mobile/components/chat/clarification-card.tsx |
CREATE | Mobile parallel, React Native primitives |
apps/web/components/chat/message-bubble.tsx |
UPDATE | Add filter for status === 'NeedsClarification'; mount <ClarificationCard> |
apps/mobile/components/message-bubble.tsx |
UPDATE | Mobile parallel |
apps/web/app/actions/chat.ts |
UPDATE | Add resolveClarification(operationId, value) server action that POSTs to the backend resolve endpoint |
apps/web/hooks/use-resolve-clarification.ts |
CREATE | TanStack mutation that calls the server action + invalidates habitKeys.lists() |
apps/mobile/hooks/use-resolve-clarification.ts |
CREATE | Mobile parallel using apiClient |
apps/web/__tests__/components/chat/clarification-card.test.tsx |
CREATE | Renders question + quick actions; tap submits + invalidates |
apps/mobile/__tests__/components/chat/clarification-card.test.tsx |
CREATE | Mobile parallel |
i18n keys
"habits": {
"clarification": {
"questionFallback": "What schedule should this be on?",
"quickAction": {
"daily": "Daily",
"weekly": "Weekly",
"xPerWeek": "{n} times per week",
"oneTime": "One-time task"
},
"submitting": "Setting up…",
"successCreated": "Created '{name}'",
"errorExpired": "This clarification expired. Please ask again.",
"errorGeneric": "Something went wrong. Please try again."
}
}Tasks
Execute in this order. Tasks 1–3 are #98 (prompt-only — ship-able alone). Tasks 4–17 are #99 backend. Tasks 18–25 are #99 frontend. Tests interleave.
Phase A — Prompt-level fix (#98)
Task 1: Tighten CreateHabitTool.Description
- Repo: api
- File:
src/Orbit.Application/Chat/Tools/Implementations/CreateHabitTool.cs - Action: UPDATE lines 22-23
- Implement: Add explicit phrase: "For one-time tasks, omit
frequency_unitONLY if the user explicitly described it as a one-time task (e.g., 'just once', 'this Friday only', 'one-time', 'uma vez'). If the user called it a habit/rotina/hábito or did not mention frequency, ASK first via clarification." - Validate:
dotnet build
Task 2: Add structuring rule for habit-flavored titles
- Repo: api
- File:
src/Orbit.Infrastructure/Services/Prompts/Sections/Static/StructuringStrategySection.cs - Action: UPDATE — insert new rule between the existing "Ask ONE targeted clarifying question when" block (line 37-43) and "NEVER ask a question (act immediately) when" block (line 45-49)
- Implement: "User calls something a 'habit' / 'rotina' / 'hábito' without stating daily / weekly / X times per week / a specific schedule → ASK before calling
create_habit. Offer: daily, weekly with specific days, X times per week, or one-time task." - Validate:
dotnet build
Task 3: Manual smoke test (post-deploy of Phase A only — optional gate)
- Verify on local dev:
- PT:
"Crie o hábito de tomar café da manhã. Eu geralmente como pão, ovos e uma porção de frutas. Gostaria que cada item desse fosse um checklist"→ AI asks before creating - EN:
"Create a meditation habit"→ AI asks - Regression:
"Create a one-time task to call the dentist Friday"→ AI creates without asking - Regression:
"Create a daily meditation habit"→ AI creates without asking
- PT:
- Skippable if Phase A and Phase B ship in the same PR.
Phase B — Backend structural fix (#99)
Task 4: Add ActionStatus.NeedsClarification + ClarificationRequest field
- Repo: api
- File:
src/Orbit.Application/Chat/Commands/ProcessUserChatCommand.cs - Action: UPDATE
- Line 47: enum becomes
{ Success, Failed, Suggestion, NeedsClarification } - Lines 38-45: add
ClarificationRequest? ClarificationRequest = nulltoActionResultrecord (afterSuggestedSubHabits)
- Line 47: enum becomes
- Validate:
dotnet build
Task 5: Define ClarificationRequest + QuickAction records
- Repo: api
- File:
src/Orbit.Application/Chat/Models/ClarificationRequest.cs(CREATE),src/Orbit.Application/Chat/Models/QuickAction.cs(CREATE) - Implement:
public record ClarificationRequest( string Question, Guid OperationId, string MissingArgumentKey, IReadOnlyList<QuickAction> QuickActions); public record QuickAction(string Label, string Value, string? Description = null);
- Validate:
dotnet build
Task 6: Create PendingClarification entity + EF config + migration
- Repo: api
- Files:
src/Orbit.Domain/Entities/PendingClarification.cs(CREATE)src/Orbit.Infrastructure/Persistence/Configurations/PendingClarificationConfiguration.cs(CREATE)src/Orbit.Infrastructure/Persistence/OrbitDbContext.cs(UPDATE — addDbSet)
- Entity fields:
Id(Guid PK),UserId(Guid, indexed),ToolName(string),PartialArgumentsJson(text),MissingArgumentKey(string),Question(string),QuickActionsJson(text),CreatedAtUtc,ExpiresAtUtc(indexed),ResolvedAtUtc? - Mirror:
src/Orbit.Domain/Entities/PendingAgentOperationState.csfor entity structure conventions - Validate:
dotnet build, thendotnet ef migrations add AddPendingClarifications --project src/Orbit.Infrastructure --startup-project src/Orbit.Api
Task 7: Implement PendingClarificationStore
- Repo: api
- Files:
src/Orbit.Application/Common/Interfaces/IPendingClarificationStore.cs(CREATE)src/Orbit.Infrastructure/Services/PendingClarificationStore.cs(CREATE)
- Implement:
Task<PendingClarification> Create(Guid userId, string toolName, JsonElement partialArgs, string missingKey, string question, IReadOnlyList<QuickAction> actions, CancellationToken ct)— setsExpiresAtUtc = DateTime.UtcNow.AddMinutes(30)Task<PendingClarification?> GetById(Guid operationId, Guid userId, CancellationToken ct)— returns null if user mismatch or expiredTask Resolve(Guid operationId, Guid userId, CancellationToken ct)— setsResolvedAtUtc, throws if already resolved
- Mirror:
src/Orbit.Infrastructure/Services/PendingAgentOperationStore.cs - Validate:
dotnet build
Task 8: Refactor CreateHabitTool to emit ClarificationRequest
- Repo: api
- File:
src/Orbit.Application/Chat/Tools/Implementations/CreateHabitTool.cs - Action: UPDATE
- Implement:
- Inject
IPendingClarificationStore+IUserDateService(already injected) + AI prompt builder for question text? Actually no — generate question + quick actions inline. - At top of
ExecuteAsync: parsetitleandfrequency_unit. - If
frequency_unit == nullAND title (lowercased) contains any of"habit","rotina","hábito":- Build
ClarificationRequestwith localized question (use title in question), 4 quick actions: Daily / Weekly / 3× per week / One-time task. - Return
ToolResult(success: true, Payload: clarificationRequest).
- Build
- Otherwise proceed with existing creation logic unchanged.
- Inject
- Validate:
dotnet build
Task 9: Wire handler to recognize ClarificationRequest payload + stash to store
- Repo: api
- File:
src/Orbit.Application/Chat/Commands/ProcessUserChatCommand.cs - Action: UPDATE — in
ExecuteSingleToolCallAsync(around lines 295-362), after the existingsuggest_breakdownspecial case (line 413), add:if (result.Payload is ClarificationRequest cr) { // OperationId is set by the store, not the tool var saved = await pendingClarificationStore.Create(userId, call.Name, ParseArgs(call.Args), cr.MissingArgumentKey, cr.Question, cr.QuickActions, ct); return new ActionResult( ToolNameToPascalCase(call.Name), ActionStatus.NeedsClarification, ClarificationRequest: cr with { OperationId = saved.Id }); }
- Inject
IPendingClarificationStoreinto the handler via the existingChatExecutionDependenciesrecord (line 73-79) - Validate:
dotnet build
Task 10: Create ResolveClarificationCommand + handler
- Repo: api
- File:
src/Orbit.Application/Chat/Commands/ResolveClarificationCommand.cs(CREATE) - Implement:
record ResolveClarificationCommand(Guid OperationId, Guid UserId, string Value) : IRequest<Result<ClarificationResolveResponse>>;- Handler:
- Load pending via
IPendingClarificationStore.GetById. If null →Result.Failure("clarification_expired_or_missing"). - Deserialize
PartialArgumentsJsontoJsonElement, mergeMissingArgumentKey: Value(useJsonObjectfor write). - Construct
AgentExecuteOperationRequestfor the originalToolName. - Call
IAgentOperationExecutor.ExecuteAsync(...). - Call
store.Resolve(...). - Return
Result<ClarificationResolveResponse>with status + entityId + entityName.
- Load pending via
- Validate:
dotnet build
Task 11: Add resolve endpoint to AiController
- Repo: api
- File:
src/Orbit.Api/Controllers/AiController.cs - Action: UPDATE — add new method:
[HttpPost("clarifications/{operationId:guid}/resolve")] public async Task<IActionResult> ResolveClarification(Guid operationId, [FromBody] ResolveClarificationRequest body, CancellationToken ct) { ... }
- Mirror: existing
Executeendpoint at line 254-296 - Validate:
dotnet build
Task 12: Add ClarificationGuidanceSection prompt section
- Repo: api
- File:
src/Orbit.Infrastructure/Services/Prompts/Sections/Static/ClarificationGuidanceSection.cs(CREATE) - Implement:
Order = 260, always include. Body teaches model:- When the user is ambiguous about a discrete enum (frequency, scope), the tool may return
NeedsClarification. - This is preferred over plain-text questions when the answer is one of a small set of choices.
- Do not over-ask. If the user gave a clear answer, do not invoke clarification.
- When the user is ambiguous about a discrete enum (frequency, scope), the tool may return
- Update: register in
SystemPromptBuilderconstructor - Validate:
dotnet build
Task 13: DI registration
- Repo: api
- File:
src/Orbit.Api/Extensions/ServiceCollectionExtensions.cs - Action: UPDATE
AddOrbitAiServices— addbuilder.Services.AddScoped<IPendingClarificationStore, PendingClarificationStore>(); - Validate:
dotnet build
Task 14: Unit tests — CreateHabitTool clarification triggers
- Repo: api
- File:
tests/Orbit.Application.Tests/Chat/Tools/CreateHabitToolClarificationTests.cs(CREATE) - Mirror:
tests/Orbit.Application.Tests/Chat/Tools/BulkLogHabitsToolTests.cs - Cases:
- Title contains "habit" + no
frequency_unit→ returns payload of typeClarificationRequest - Title contains "hábito" + no
frequency_unit→ same - Title contains "rotina" + no
frequency_unit→ same - Title contains "habit" +
frequency_unit = "Day"→ creates habit (no clarification) - Title = "call the dentist" + no
frequency_unit→ creates one-time task (no clarification) - Title case sensitivity: "MY DAILY HABIT" + no
frequency_unit→ returns clarification
- Title contains "habit" + no
- Validate:
dotnet test tests/Orbit.Application.Tests --filter CreateHabitToolClarificationTests
Task 15: Unit tests — ResolveClarificationCommandHandler
- Repo: api
- File:
tests/Orbit.Application.Tests/Chat/Commands/ResolveClarificationCommandTests.cs(CREATE) - Cases: happy path; expired returns failure; already-resolved returns failure; wrong-user returns failure; missing arg key handled
- Validate:
dotnet test
Task 16: Integration test — full clarification flow
- Repo: api
- File:
tests/Orbit.IntegrationTests/Chat/ClarificationFlowTests.cs(CREATE) - Mirror: existing integration tests under
tests/Orbit.IntegrationTests/(find Habits or Chat folder) - Flow:
- POST
/api/chatwith body{ message: "Create a meditation habit" } - Assert response contains
actions[0].status === "NeedsClarification"+clarificationRequest.operationId - POST
/api/ai/clarifications/{operationId}/resolvewith{ value: "Day" } - Assert habit is created with
FrequencyUnit = Day, FrequencyQuantity = 1 - Second resolve call returns 410 Gone
- POST
- Validate:
dotnet test tests/Orbit.IntegrationTests --filter ClarificationFlowTests
Task 17: Manual smoke (backend only — optional)
- Start API locally, hit
/api/chatwith bearer token, confirmNeedsClarificationis returned for "Create a meditation habit" but UI not yet wired. - Skippable if Phase C is merged together.
Phase C — Frontend structural fix (#99) — PARITY REQUIRED
Task 18: Extend shared Zod chat types
- Repo: ui-mobile
- File:
packages/shared/src/types/chat.ts - Action: UPDATE
actionStatusSchema = z.enum(['Success', 'Failed', 'Suggestion', 'NeedsClarification'])- Add
quickActionSchema = z.object({ label, value, description: z.string().optional() }) - Add
clarificationRequestSchema = z.object({ question, operationId: z.string().uuid(), missingArgumentKey, quickActions: z.array(quickActionSchema) }) - Add
clarificationRequest: clarificationRequestSchema.optional()toactionResultSchema
- Validate:
npm run type-check
Task 19: Add resolve endpoint constant
- Repo: ui-mobile
- File:
packages/shared/src/api/endpoints.ts - Action: UPDATE — add under
ai:clarifications: { resolve: (operationId: string) => `/api/ai/clarifications/${operationId}/resolve` as const, }
- Validate:
npm run type-check
Task 20: Add i18n keys
- Repo: ui-mobile
- Files:
packages/shared/src/i18n/en.json,packages/shared/src/i18n/pt-BR.json - Action: UPDATE — add
habits.clarification.*namespace (see "i18n keys" above) - Validate:
npm run type-check
Task 21: Web — resolveClarification server action
- Repo: ui-mobile
- File:
apps/web/app/actions/chat.ts - Action: UPDATE — add async function
resolveClarification(operationId: string, value: string)that POSTs to${API_BASE}/api/ai/clarifications/${operationId}/resolvewith bearer cookie. Returns parsedClarificationResolveResponse. - Mirror: existing
sendChatMessageat line ~37 - Validate:
npm run type-check
Task 22: Web hook — useResolveClarification
- Repo: ui-mobile
- File:
apps/web/hooks/use-resolve-clarification.ts(CREATE) - Action: CREATE —
useMutationcalling the server action; on success:queryClient.invalidateQueries({ queryKey: habitKeys.lists() }) - Mirror:
apps/web/hooks/use-habits.tsmutation patterns - Validate:
npm run type-check
Task 23: Web <ClarificationCard> component
- Repo: ui-mobile
- File:
apps/web/components/chat/clarification-card.tsx(CREATE) - Implement:
- Props:
{ clarificationRequest, onResolved, onCancelled } - Shows
clarificationRequest.question+ map quick actions to pill buttons - Tap →
useResolveClarification.mutateAsync({ operationId, value: button.value }) - States: idle, submitting (spinner on tapped button), success (green check + entity name), error (red text + retry)
- Visual:
bg-surface-elevated/50 border border-border-muted rounded-[var(--radius-xl)] p-4 shadow-[var(--shadow-sm)] - Quick action button:
bg-surface-elevated border border-border-muted rounded-full px-3 py-1.5 text-xs(matchingsuggestion-chips.tsxstyling)
- Props:
- Mirror:
apps/web/components/chat/breakdown-suggestion.tsx - Validate:
npm run type-check && npm run lint
Task 24: Web dispatch — message-bubble.tsx
- Repo: ui-mobile
- File:
apps/web/components/chat/message-bubble.tsx - Action: UPDATE
- Add filter:
const clarificationActions = actions.filter(a => a.status === 'NeedsClarification' && a.clarificationRequest) - Update
nonSuggestionActionsfilter to also excludeNeedsClarification - Mount
<ClarificationCard>for eachclarificationActionsentry (similar to existing<BreakdownSuggestion>mount)
- Add filter:
- Validate:
npm run type-check && npm run lint
Task 25: Mobile — hook + card + dispatch (parity with 21-24)
- Repo: ui-mobile
- Files:
apps/mobile/hooks/use-resolve-clarification.ts(CREATE) — usesapiClientinstead of server actionapps/mobile/components/chat/clarification-card.tsx(CREATE) — React Native primitives, matchesapps/mobile/components/chat/breakdown-suggestion.tsxstyling tokensapps/mobile/components/message-bubble.tsx(UPDATE) — parallel dispatch
- Validate:
npm run type-check && npm run lint
Task 26: Web unit tests
- Repo: ui-mobile
- File:
apps/web/__tests__/components/chat/clarification-card.test.tsx(CREATE) - Mirror:
apps/web/__tests__/components/chat/breakdown-suggestion.test.tsx - Cases: renders question + buttons; tap submits; loading state; success state; expired-error state
- Validate:
npx vitest run apps/web/__tests__/components/chat/clarification-card.test.tsx
Task 27: Mobile unit tests
- Repo: ui-mobile
- File:
apps/mobile/__tests__/components/chat/clarification-card.test.tsx(CREATE) - Mirror: any existing mobile chat tests (e.g.
apps/mobile/__tests__/components/chat/action-chips.test.tsx) - Validate:
npx vitest run apps/mobile/__tests__/components/chat/clarification-card.test.tsx
Validation commands
orbit-api (from C:\Users\thoma\Documents\Programming\Projects\orbit-api)
dotnet build
dotnet test tests/Orbit.Application.Tests
dotnet test tests/Orbit.IntegrationTests --filter Clarificationorbit-ui-mobile (from C:\Users\thoma\Documents\Programming\Projects\orbit-ui-mobile)
npm run lint
npm run type-check
npx vitest runEnd-to-end smoke
- Start API:
dotnet run --project src/Orbit.Apiin orbit-api - Start web:
npm run webin orbit-ui-mobile - Log in as test user, open
/chat - Send: "Create a meditation habit" →
<ClarificationCard>renders with Daily / Weekly / 3× per week / One-time buttons - Tap "Daily" → card shows "Created 'meditation'" success state; habit appears in
/habitswithfrequency_unit: Day, frequency_quantity: 1 - Send (regression): "Create a one-time task to call the dentist Friday" → habit created immediately, no card
- Send (regression): "Create a daily meditation habit" → habit created immediately, no card
- Send PT: "Crie o hábito de tomar café da manhã" → ClarificationCard renders
- Repeat all of the above on mobile via
npm run android - Open
/chatagain 31 minutes after clarification rendered — tap a button → see "expired" error (validates TTL)
Risks
| Risk | Mitigation |
|---|---|
Model returns NeedsClarification for cases where the user already gave clear intent (over-asking) |
New prompt section explicitly tells the model not to over-ask. Integration test covers the regression-safe cases. |
| Tool heuristic too narrow (verb-phrase habits like "Tomar café") | Two-layer defense: prompt rule from #98 catches it before the tool fires. Tool is the safety net for explicit habit-keyword cases. |
PendingClarification table grows unbounded |
TTL of 30 min via ExpiresAtUtc; add a hosted background sweep job in a follow-up (out of scope here — note in PR description). |
| Concurrent resolve attempts (double-tap) | Resolve() is one-shot — throws on second call. Frontend disables button after first tap; error state catches race. |
| EF migration on Render auto-deploy could fail mid-rollout if web ships first | Backend ships and migration runs before frontend can hit the new endpoint. Sequence: merge api PR first → wait for Render deploy → merge ui-mobile PR. |
| Shared types out of sync with backend record | Zod schemas in packages/shared/src/types/chat.ts are hand-mirrored from C# — manual diligence required. Add a snapshot test that asserts the shape doesn't drift (future). |
Acceptance criteria
- All tasks (1-27) completed
-
dotnet build+dotnet testpass on orbit-api -
npm run lint+npm run type-check+npx vitest runpass on orbit-ui-mobile - Parity verified: web + mobile
<ClarificationCard>behave identically (E2E checklist) - EF migration applied locally + runs clean
- Manual smoke checklist passes including PT and EN cases, regression-safe paths, and TTL expiry
- No new
console.logs in frontend - Zero
anytypes in new TS files - All user-facing strings i18n'd (no hardcoded English in components)
- One branch per repo:
fix/chat-clarificationin both - PR descriptions cross-link issues AI silently creates one-time tasks when habit frequency isn't specified #98 and Add ActionStatus.NeedsClarification + chat UI question-card (web + mobile) #99 and the sibling repo's PR
3 tasks
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.
Summary
Frontend half of the two-layer #98 + #99 fix. Renders a structured
<ClarificationCard>with quick-action buttons (Daily, Weekly, 3× per week, One-time) when the AI returnsNeedsClarificationfor a habit-flavored title with no frequency. Tapping a button POSTs to the new backend resolve endpoint, which merges the chosen value into the partial args and re-invokescreate_habitdeterministically server-side.actionStatusSchemawith'NeedsClarification'; addsquickActionSchema,clarificationRequestSchema,clarificationResolveResponseSchema; addsclarificationRequestfield toactionResultSchema; addsAPI.ai.clarificationResolve(operationId); addshabits.clarification.*i18n keys inen.jsonandpt-BR.jsonresolveClarificationserver action;useResolveClarificationTanStack mutation hook (invalidateshabitKeys.lists/count/summaryPrefix);<ClarificationCard>component with idle/submitting/success/error states; dispatch wired inmessage-bubble.tsxuseResolveClarification(apiClient-based);<ClarificationCard>(React Native + StyleSheet matchingbreakdown-suggestion.tsxvisual tokens); dispatch wired inmessage-bubble.tsxLinked issues
Closes #98
Closes #99
Paired PR
Parity
apps/web/hooks/use-resolve-clarification.tsapps/mobile/hooks/use-resolve-clarification.tsapps/web/components/chat/clarification-card.tsxapps/mobile/components/chat/clarification-card.tsxapps/web/components/chat/message-bubble.tsxapps/mobile/components/message-bubble.tsxTest plan
npm run type-check(turbo, all 3 packages): PASSnpm test: PASS — 1549 web + mobile + shared tests, 10 new clarification card testsfrequency_unit: Day, frequency_quantity: 1npm run androidNotes
npm run lintis broken onmain:next lintwas removed in Next.js 15+, ESLint 10 needs a new flat-config format. Verified to be a pre-existing issue, not caused by this PR. Tooling-modernization fix belongs in a separate PR.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests
Localization