fix(wren-ui): normalize Instruction timestamp serialization to fix #2073#2165
Open
KerberosClaw wants to merge 2 commits intoCanner:mainfrom
Open
fix(wren-ui): normalize Instruction timestamp serialization to fix #2073#2165KerberosClaw wants to merge 2 commits intoCanner:mainfrom
KerberosClaw wants to merge 2 commits intoCanner:mainfrom
Conversation
) Add nested resolver for Instruction type to convert createdAt/updatedAt to ISO string, matching the pattern used by SqlPair and ApiHistoryResponse. Without this, PostgreSQL returns Date objects that graphql-js serializes via Date.valueOf() into epoch milliseconds strings, which dayjs misparses as historical dates (e.g., 1770-11-08 instead of 2025-12-04). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdded a GraphQL nested resolver for the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@wren-ui/src/apollo/server/resolvers/instructionResolver.ts`:
- Around line 95-103: The createdAt and updatedAt field resolvers in
instructionResolver.ts return null for falsy timestamps which violates the
schema's non-nullable String!; update the createdAt and updatedAt resolver
functions (the createdAt: (instruction: Instruction) => { ... } and updatedAt:
(instruction: Instruction) => { ... } implementations) to always return a valid
ISO string — e.g. if instruction.createdAt or instruction.updatedAt is falsy,
return new Date().toISOString() (or another deterministic fallback ISO string)
instead of null so the resolvers never return null for the non-nullable GraphQL
fields.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 9429a695-b85e-406f-8df8-b9179be40238
📒 Files selected for processing (2)
wren-ui/src/apollo/server/resolvers.tswren-ui/src/apollo/server/resolvers/instructionResolver.ts
Comment on lines
+95
to
+103
| return instruction.createdAt | ||
| ? new Date(instruction.createdAt).toISOString() | ||
| : null; | ||
| }, | ||
| updatedAt: (instruction: Instruction) => { | ||
| return instruction.updatedAt | ||
| ? new Date(instruction.updatedAt).toISOString() | ||
| : null; | ||
| }, |
Contributor
There was a problem hiding this comment.
Avoid returning null for non-nullable GraphQL timestamp fields.
Instruction.createdAt / updatedAt are non-nullable in schema (String!), but this resolver returns null on falsy values, which can cause runtime GraphQL nullability errors.
💡 Suggested fix
+ private normalizeInstructionTimestamp(value: unknown, fieldName: string): string {
+ if (value == null) {
+ throw new Error(`Instruction.${fieldName} is required`);
+ }
+
+ const date = new Date(value as string | number | Date);
+ if (Number.isNaN(date.getTime())) {
+ throw new Error(`Invalid Instruction.${fieldName}: ${String(value)}`);
+ }
+
+ return date.toISOString();
+ }
+
public getInstructionNestedResolver = () => ({
createdAt: (instruction: Instruction) => {
- return instruction.createdAt
- ? new Date(instruction.createdAt).toISOString()
- : null;
+ return this.normalizeInstructionTimestamp(
+ instruction.createdAt,
+ 'createdAt',
+ );
},
updatedAt: (instruction: Instruction) => {
- return instruction.updatedAt
- ? new Date(instruction.updatedAt).toISOString()
- : null;
+ return this.normalizeInstructionTimestamp(
+ instruction.updatedAt,
+ 'updatedAt',
+ );
},
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| return instruction.createdAt | |
| ? new Date(instruction.createdAt).toISOString() | |
| : null; | |
| }, | |
| updatedAt: (instruction: Instruction) => { | |
| return instruction.updatedAt | |
| ? new Date(instruction.updatedAt).toISOString() | |
| : null; | |
| }, | |
| private normalizeInstructionTimestamp(value: unknown, fieldName: string): string { | |
| if (value == null) { | |
| throw new Error(`Instruction.${fieldName} is required`); | |
| } | |
| const date = new Date(value as string | number | Date); | |
| if (Number.isNaN(date.getTime())) { | |
| throw new Error(`Invalid Instruction.${fieldName}: ${String(value)}`); | |
| } | |
| return date.toISOString(); | |
| } | |
| public getInstructionNestedResolver = () => ({ | |
| createdAt: (instruction: Instruction) => { | |
| return this.normalizeInstructionTimestamp( | |
| instruction.createdAt, | |
| 'createdAt', | |
| ); | |
| }, | |
| updatedAt: (instruction: Instruction) => { | |
| return this.normalizeInstructionTimestamp( | |
| instruction.updatedAt, | |
| 'updatedAt', | |
| ); | |
| }, | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@wren-ui/src/apollo/server/resolvers/instructionResolver.ts` around lines 95 -
103, The createdAt and updatedAt field resolvers in instructionResolver.ts
return null for falsy timestamps which violates the schema's non-nullable
String!; update the createdAt and updatedAt resolver functions (the createdAt:
(instruction: Instruction) => { ... } and updatedAt: (instruction: Instruction)
=> { ... } implementations) to always return a valid ISO string — e.g. if
instruction.createdAt or instruction.updatedAt is falsy, return new
Date().toISOString() (or another deterministic fallback ISO string) instead of
null so the resolvers never return null for the non-nullable GraphQL fields.
Instruction.createdAt and updatedAt are String! (non-nullable) in the GraphQL schema, so the resolver should not return null. Follow the SqlPairResolver pattern which also omits null checks. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
createdAt/updatedAtdisplays incorrect historical dates (e.g.,1770-11-08) on the Knowledge → Instructions page when using PostgreSQLinstructionResolverwas missing a nested resolver for timestamp fields. When PostgreSQL returnsDateobjects,graphql-jsserializes them viaDate.valueOf()→ epoch milliseconds string, whichdayjsmisparses as a historical yeargetInstructionNestedResolver()toInstructionResolver, following the same pattern used bySqlPairResolverandApiHistoryResolverTest plan
graphql@16.8.1+dayjs@1.11.13in Docker:Dateobject → without fix:dayjs("1764816830199")→1770-11-08(WRONG)Dateobject → with fix:dayjs("2025-12-04T02:53:50.199Z")→2025-12-04(CORRECT)CURRENT_TIMESTAMPformat → with fix: correctly normalized to ISOnull→ returnsnull(safe)SqlPairResolverandApiHistoryResolveruse identical patternyarn build(pre-existingvega-litetype error onmain), so no regression introducedSummary by CodeRabbit