feat(Productivity App): Replace locale selector in productivity app example by a duration selector#628
Conversation
| duration: z | ||
| .number() | ||
| .min(1) | ||
| .max(4) | ||
| .optional() | ||
| .default(1) | ||
| .describe( | ||
| "Number of weeks to aggregate (1 = one week, 2 = two weeks, 4 = four weeks)", | ||
| ), |
There was a problem hiding this comment.
Schema allows
duration: 3 but UI only handles 1, 2, 4
z.number().min(1).max(4) permits any integer 1–4, including 3. The description text even reinforces the intended values ("1 = one week, 2 = two weeks, 4 = four weeks"), but nothing in the schema actually restricts the AI to those three values. If the model passes duration: 3, the aggregation in getWeeks works correctly, but the frontend dropdown (value={widgetState.duration}) has no matching <option> and will render with a blank/default-selected state.
Consider using a z.union of literals to enforce the exact set:
duration: z
.union([z.literal(1), z.literal(2), z.literal(4)])
.optional()
.default(1)
.describe("Number of weeks to aggregate (1, 2, or 4)"),Prompt To Fix With AI
This is a comment left during a code review.
Path: examples/productivity/server/src/index.ts
Line: 105-113
Comment:
**Schema allows `duration: 3` but UI only handles 1, 2, 4**
`z.number().min(1).max(4)` permits any integer 1–4, including `3`. The description text even reinforces the intended values ("1 = one week, 2 = two weeks, 4 = four weeks"), but nothing in the schema actually restricts the AI to those three values. If the model passes `duration: 3`, the aggregation in `getWeeks` works correctly, but the frontend dropdown (`value={widgetState.duration}`) has no matching `<option>` and will render with a blank/default-selected state.
Consider using a `z.union` of literals to enforce the exact set:
```ts
duration: z
.union([z.literal(1), z.literal(2), z.literal(4)])
.optional()
.default(1)
.describe("Number of weeks to aggregate (1, 2, or 4)"),
```
How can I resolve this? If you propose a fix, please make it concise.| - **Theme Support**: Adapts to light/dark mode using the `useLayout()` hook | ||
| - **Localization**: Translates UI based on user locale via `useUser()` hook (English, French, Spanish, Chinese) | ||
| - **Localization**: Automatically translates UI based on user locale via `useUser()` hook (English, French, Spanish, Chinese) — no manual selector needed | ||
| - **Duration Selection**: Aggregates productivity data across 1, 2, or 4 weeks using a header dropdown, powered by the `duration` tool parameter |
There was a problem hiding this comment.
Not relevant - it should only showcase the skybridge APIs
| # Productivity Example | ||
|
|
||
| An example MCP app built with [Skybridge](https://docs.skybridge.tech/home): interactive productivity charts with localization, display mode, follow-up messages, and widget-to-tool communication. | ||
| An example MCP app built with [Skybridge](https://docs.skybridge.tech/home): interactive productivity charts with duration selection, localization, display mode, follow-up messages, and widget-to-tool communication. |
qchuchu
left a comment
There was a problem hiding this comment.
A few comments to improve. You can also rebase - we had an issue on a non up to date lock file. Thanks for contributing !
…ts.tsx Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Closes #366
Changes
skybridge\examples\productivity\server\src\index.tsserver/src/index.tsfor duration select
Result
Before
After
I used Claude Code heavily for this PR.
Greptile Summary
This PR replaces the manual locale selector in the productivity widget with a duration selector (1, 2, or 4 weeks), and simplifies locale handling by deriving it directly from
useUser().localeinstead of maintaining mutable state. ThegetWeeks()aggregation logic on the server is well-implemented and correctly averages daily activity hours across multiple weeks.Key findings:
useEffectsync guard uses a ref (lastSyncedInputOffset) that only tracksweekOffset. If the AI directly calls the tool with the sameweekOffsetbut a differentduration, the guard prevents the updated output from being written towidgetState, leaving the widget showing stale data. The dropdown path (goToWeek→onSuccess) is correct; only the direct-call sync path is broken.duration: 3: The Zod schema uses.min(1).max(4), which allows the value3. The UI only offers[1, 2, 4], so a tool call withduration: 3(valid per the schema) would result in the dropdown rendering with no matching option. Restricting the schema to a union of literals (1 | 2 | 4) would align the server contract with the UI.<select>still carriesclassName=\"lang-select\"from the old locale selector.Confidence Score: 4/5
Safe to merge after addressing the stale sync bug that prevents widget state updates when the AI changes only the duration on the same weekOffset.
One P1 logic bug: the sync guard in useEffect only checks weekOffset, so AI-driven duration changes on the same offset silently drop the new output. The remaining issues (schema literal set, stale CSS class) are P2. The aggregation logic itself is correct and the i18n.ts simplification is clean.
examples/productivity/web/src/widgets/show-productivity-insights.tsx — the useEffect sync guard needs to track both weekOffset and duration.
Important Files Changed
Comments Outside Diff (1)
examples/productivity/web/src/widgets/show-productivity-insights.tsx, line 54-59 (link)The
lastSyncedInputOffsetref only guards onweekOffset. When the AI directly re-invokes the tool with the sameweekOffsetbut a differentduration(e.g. going from{weekOffset: 0, duration: 1}to{weekOffset: 0, duration: 2}), the guardlastSyncedInputOffset.current !== weekOffsetevaluates to0 !== 0 → false, so the effect exits early andwidgetStateis never updated with the new aggregated output. The widget continues showing stale data from the previous call.The
onSuccesspath ingoToWeekhandles the dropdown interaction correctly, but this effect is the only mechanism for syncing state when the AI directly calls the tool — and it is now blind to duration changes.Consider tracking
durationalongsideweekOffsetin the ref so the guard fires whenever either navigation parameter changes:Prompt To Fix With AI
Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "Update docs" | Re-trigger Greptile