-
Notifications
You must be signed in to change notification settings - Fork 2
Analyze staging environment errors on Railway #7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Analyze staging environment errors on Railway #7
Conversation
- Fix ownerId type mismatch in projects.ts (null -> empty string) - Add entityId to NPC portrait and quest banner generation calls - Change imageData to imageUrl with data URL format in save-portrait calls - Fix import.meta.env.DEV comparison (string -> boolean) - Register TypeBox format validators for email, uuid, uri, date-time - Fix Optional type tests to use object context - Add server availability check to load tests to skip when server not running
WalkthroughThe PR adds server availability checks to load tests, updates content generation APIs to include entityId parameters, switches portrait transmission from base64 imageData to imageUrl data URLs, refactors validation tests for object-context schema validation, and corrects environment checks from string to boolean comparisons. Changes
Sequence Diagram(s)sequenceDiagram
participant Component as Component<br/>(ContentPreviewCard)
participant API as Backend API
participant Storage as Storage
rect rgb(220, 240, 255)
Note over Component,Storage: Old Flow: Base64 imageData
Component->>API: generate-npc-portrait<br/>(npcName, archetype,<br/>appearance, personality)
API-->>Component: portrait base64
Component->>API: save-portrait<br/>(entityType, entityId,<br/>imageData: base64)
API->>Storage: Store base64
end
rect rgb(240, 255, 240)
Note over Component,Storage: New Flow: Data URL imageUrl + entityId
Component->>API: generate-npc-portrait<br/>(npcName, entityId,<br/>archetype, appearance,<br/>personality)
API-->>Component: portrait base64
Component->>Component: Encode to data URL
Component->>API: save-portrait<br/>(entityType, entityId,<br/>imageUrl: data:image/...)
API->>Storage: Store data URL
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ 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 |
Summary of ChangesHello @Dexploarer, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request primarily focuses on improving the robustness of the application's testing suite and refining data handling for image generation and storage. It introduces a mechanism to conditionally skip load tests if the server is unavailable, enhances TypeBox schema validation with explicit format checks, and corrects how optional types are tested. Additionally, it standardizes image data formats for API calls and addresses minor type mismatches and environment variable comparisons to ensure more reliable and consistent behavior across the application. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
PR Compliance Guide 🔍(Compliance updated until commit 7ef877c)Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label Previous compliance checksCompliance check up to commit 7ef877c
|
||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||
|
|
||
| autocannon.track(instance); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Issue: Potential Resource Leak in runLoad
The use of autocannon.track(instance) within the promise does not guarantee that the instance is properly cleaned up after the test completes. If the instance is not destroyed or closed, it may lead to resource exhaustion, especially when running multiple tests in sequence or under high concurrency.
Recommendation:
Ensure that the autocannon instance is properly destroyed after the test completes. You can do this by calling instance.stop() or similar cleanup logic in the callback, regardless of success or error:
const instance = autocannon(options, (err, result) => {
instance.stop(); // Ensure cleanup
if (err) {
reject(err);
} else {
resolve(result);
}
});This will help prevent resource leaks and improve test reliability.
| FormatRegistry.Set("email", (value) => | ||
| /^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$/.test(value), | ||
| ); | ||
| FormatRegistry.Set("uuid", (value) => | ||
| /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i.test( | ||
| value, | ||
| ), | ||
| ); | ||
| FormatRegistry.Set("uri", (value) => { | ||
| try { | ||
| new URL(value); | ||
| return true; | ||
| } catch { | ||
| return false; | ||
| } | ||
| }); | ||
| FormatRegistry.Set("date-time", (value) => { | ||
| const date = new Date(value); | ||
| return !isNaN(date.getTime()) && value.includes("T"); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Format Validators May Not Fully Cover Edge Cases
The custom format validators for email, uuid, uri, and date-time use regular expressions and logic that do not fully comply with their respective standards (e.g., RFC 5322 for email). This can lead to false positives or negatives in validation, reducing the reliability of the test suite. For example, the email regex is simplistic and may not handle all valid/invalid emails, and the URI validator may accept non-HTTP/HTTPS schemes.
Recommendation: Consider using more robust validation libraries (such as validator.js) for format registration, or document the limitations of these validators to avoid confusion about what is actually being tested.
| it("should handle optional types in object context", () => { | ||
| // Optional works correctly within object schemas | ||
| const schema = t.Object({ | ||
| name: t.Optional(t.String()), | ||
| }); | ||
|
|
||
| expect(Value.Check(schema, "value")).toBe(true); | ||
| expect(Value.Check(schema, undefined)).toBe(true); | ||
| expect(Value.Check(schema, null)).toBe(false); | ||
| expect(Value.Check(schema, { name: "value" })).toBe(true); | ||
| expect(Value.Check(schema, {})).toBe(true); | ||
| expect(Value.Check(schema, { name: null })).toBe(false); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Optional and Nullable Type Handling Is Version-Dependent
The tests for optional and nullable types in object context (lines 364-373) are correct for the current TypeBox behavior, but TypeBox's handling of t.Optional and t.Null can be subtle and may change in future versions. If TypeBox changes its semantics, these tests could become misleading or fail unexpectedly.
Recommendation: Monitor TypeBox release notes for changes in optional/nullable semantics and update these tests accordingly to ensure continued correctness.
| name: body.name, | ||
| description: body.description, | ||
| ownerId: user?.id || null, // Single-team: ownerId is optional | ||
| ownerId: user?.id || "", // Single-team: ownerId is optional |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ambiguous Ownership Handling
Passing ownerId: user?.id || "" may result in projects with an empty string as the owner if the user is unauthenticated. This can lead to ambiguous or invalid ownership records in the database. It is recommended to explicitly set ownerId to null (or omit the field) when unauthenticated, and ensure the service layer and database schema handle this case appropriately:
ownerId: user?.id ?? nullThis approach makes the intent clearer and avoids potential issues with empty string values.
| settings: body.settings, | ||
| metadata: body.metadata, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lack of Validation for Arbitrary Object Fields
The settings and metadata fields are accepted as arbitrary objects (t.Record(t.String(), t.Unknown())) and passed directly to the service. Without additional validation or sanitization, this could allow storage of unexpected or malicious data. It is advisable to implement stricter validation or sanitization for these fields, or ensure the service layer enforces constraints to prevent injection or data integrity issues.
| const base64Image = base64data.split(",")[1]; // Remove data:image/png;base64, prefix | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Issue: Base64 Extraction Safety
The code assumes that base64data.split(",")[1] will always yield a valid base64 string. If reader.result is not a valid data URL, this will be undefined, potentially causing the backend to receive invalid data.
Recommendation:
Validate that the split result contains the expected base64 string before proceeding. If not, handle the error gracefully.
const parts = base64data.split(",");
if (parts.length < 2 || !parts[1]) {
notify.error("Failed to extract base64 image data");
setIsSavingPortrait(false);
return;
}
const base64Image = parts[1];| } else { | ||
| // Other errors - retry if we haven't exceeded max retries | ||
| if (retryCount < maxRetries) { | ||
| console.log(`[LibraryCard] Retrying portrait fetch for ${item.name} (attempt ${retryCount + 1}/${maxRetries})`); | ||
| setTimeout(() => { | ||
| fetchPortrait(retryCount + 1); | ||
| }, retryDelay * (retryCount + 1)); // Exponential backoff | ||
| console.log( | ||
| `[LibraryCard] Retrying portrait fetch for ${item.name} (attempt ${retryCount + 1}/${maxRetries})`, | ||
| ); | ||
| setTimeout( | ||
| () => { | ||
| fetchPortrait(retryCount + 1); | ||
| }, | ||
| retryDelay * (retryCount + 1), | ||
| ); // Exponential backoff | ||
| } else { | ||
| console.warn(`[LibraryCard] Failed to fetch portrait for ${item.name} after ${maxRetries} retries:`, response.status); | ||
| console.warn( | ||
| `[LibraryCard] Failed to fetch portrait for ${item.name} after ${maxRetries} retries:`, | ||
| response.status, | ||
| ); | ||
| // Don't clear existing URL on final failure - preserve what we have | ||
| } | ||
| } | ||
| } catch (error) { | ||
| // Network errors - retry if we haven't exceeded max retries | ||
| if (retryCount < maxRetries) { | ||
| console.log(`[LibraryCard] Network error, retrying portrait fetch for ${item.name} (attempt ${retryCount + 1}/${maxRetries})`); | ||
| setTimeout(() => { | ||
| fetchPortrait(retryCount + 1); | ||
| }, retryDelay * (retryCount + 1)); // Exponential backoff | ||
| console.log( | ||
| `[LibraryCard] Network error, retrying portrait fetch for ${item.name} (attempt ${retryCount + 1}/${maxRetries})`, | ||
| ); | ||
| setTimeout( | ||
| () => { | ||
| fetchPortrait(retryCount + 1); | ||
| }, | ||
| retryDelay * (retryCount + 1), | ||
| ); // Exponential backoff | ||
| } else { | ||
| console.error(`[LibraryCard] Error fetching portrait for ${item.name} after ${maxRetries} retries:`, error); | ||
| console.error( | ||
| `[LibraryCard] Error fetching portrait for ${item.name} after ${maxRetries} retries:`, | ||
| error, | ||
| ); | ||
| // Don't clear existing URL on final failure - preserve what we have | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential Data Race and Memory Leak in Retry Logic
The retry logic in fetchPortrait (and similarly in fetchBanner) uses setTimeout to schedule retries, but does not cancel pending timeouts if the component unmounts or if a new fetch is triggered (e.g., due to prop changes). This can lead to state updates on unmounted components, causing React warnings and potential memory leaks.
Recommended Solution:
- Use a
useRefto track whether the component is mounted, and check this before callingsetPortraitUrlorsetBannerUrl. - Alternatively, use an
AbortControllerto cancel fetch requests and associated retries when the component unmounts or when a new fetch is initiated. - Example:
useEffect(() => { let isMounted = true; fetchPortrait(); return () => { isMounted = false; }; }, [item.id, item.type, refreshKey]); // Then check isMounted before updating state in fetchPortrait
| if (token) { | ||
| headers["Authorization"] = `Bearer ${token}`; | ||
| } | ||
|
|
||
| const response = await fetch( | ||
| `/api/content/media/${item.type}/${item.id}`, | ||
| { headers } | ||
| { headers }, | ||
| ); | ||
| if (response.ok) { | ||
| const data = await response.json(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Security: Unvalidated Media URLs Used in DOM
The code sets portraitUrl and bannerUrl directly from API responses and uses them as the src attribute for <img> tags. If the backend is compromised or misconfigured, this could allow injection of malicious URLs, potentially leading to XSS or other security issues.
Recommended Solution:
- Validate or sanitize URLs before using them in the DOM. Ensure that only trusted domains or expected URL formats are allowed.
- Example:
const isValidUrl = (url: string) => url.startsWith('https://your-cdn-domain.com/'); if (url && isValidUrl(url)) { setPortraitUrl(url); }
| private enabled: boolean; | ||
|
|
||
| constructor() { | ||
| this.enabled = import.meta.env.DEV === "true"; | ||
| this.enabled = import.meta.env.DEV === true; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fragile environment check for enabling performance monitoring
The constructor uses this.enabled = import.meta.env.DEV === true;, which strictly checks for a boolean true. If import.meta.env.DEV is set as a string (e.g., 'true'), this will evaluate to false, unintentionally disabling performance monitoring in development.
Recommended solution:
Use a more robust check, such as:
this.enabled = import.meta.env.DEV === true || import.meta.env.DEV === 'true';Or coerce to boolean:
this.enabled = Boolean(import.meta.env.DEV);This ensures performance monitoring is enabled regardless of the environment variable's type.
| if (import.meta.env.DEV !== true) { | ||
| return { markRender: () => {}, measureRender: () => {} }; | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strict environment check may disable hook in development
The hook uses if (import.meta.env.DEV !== true) to determine if it should return no-op functions. This strict check will fail if import.meta.env.DEV is a string (e.g., 'true'), causing the hook to be disabled even in development environments.
Recommended solution:
Use a more flexible check:
if (!(import.meta.env.DEV === true || import.meta.env.DEV === 'true')) { ... }Or coerce to boolean:
if (!Boolean(import.meta.env.DEV)) { ... }This will ensure the hook works as intended in all development configurations.
Greptile OverviewGreptile SummaryThis PR addresses multiple staging environment errors discovered on Railway by fixing type mismatches, API parameter inconsistencies, and test reliability issues. The changes include correcting environment variable comparisons where Important Files Changed
Confidence score: 4/5
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
6 files reviewed, 2 comments
| ); | ||
| const result = await api.api.content["generate-npc-portrait"].post({ | ||
| npcName: npc.name, | ||
| entityId: item.id!, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Non-null assertion used without checking if item.id exists. Should there be a guard clause to ensure item.id exists before making this API call?
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/core/src/components/content/LibraryCard.tsx
Line: 294:294
Comment:
**logic:** Non-null assertion used without checking if item.id exists. Should there be a guard clause to ensure item.id exists before making this API call?
How can I resolve this? If you propose a fix, please make it concise.| ); | ||
| const result = await api.api.content["generate-quest-banner"].post({ | ||
| questTitle: quest.title, | ||
| entityId: item.id!, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Non-null assertion used without checking if item.id exists. Should there be a guard clause to ensure item.id exists before making this API call?
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/core/src/components/content/LibraryCard.tsx
Line: 439:439
Comment:
**logic:** Non-null assertion used without checking if item.id exists. Should there be a guard clause to ensure item.id exists before making this API call?
How can I resolve this? If you propose a fix, please make it concise.
CI Feedback 🧐A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
PR Code Suggestions ✨
Explore these optional code suggestions:
|
|||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request delivers a solid set of bug fixes and enhancements. The corrections for the ownerId type mismatch and the import.meta.env.DEV boolean comparison are crucial for application stability. The updates to the validation tests, which now correctly handle optional types and register format validators, significantly improve test accuracy. Furthermore, enhancing the content generation APIs to include entityId and use standard data URLs is a good step forward for data consistency. My main feedback is a suggestion to refactor the load test skipping logic to use bun:test's idiomatic it.if() or it.skipIf() constructs, which would simplify the test code and improve clarity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ 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".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/core/__tests__/integration/validation.test.ts (1)
15-36: Good addition of format validators for comprehensive testing.The format validators enable TypeBox Value.Check to validate string formats (email, uuid, uri, date-time) which are not validated by default. The patterns look reasonable, though consider the following:
For the UUID regex (lines 20-24), consider simplifying to accept all UUID versions or add a comment explaining why only v1-v5 are accepted:
FormatRegistry.Set("uuid", (value) => - /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i.test( + /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i.test( value, ), );The current pattern restricts to UUID v1-v5 in the version field (
[1-5]) and variant field ([89ab]). If this is intentional, add a comment; otherwise, the simplified version accepts all valid UUID formats.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/core/__tests__/integration/load.test.ts(22 hunks)apps/core/__tests__/integration/validation.test.ts(3 hunks)apps/core/server/routes/projects.ts(1 hunks)apps/core/src/components/content/ContentPreviewCard.tsx(2 hunks)apps/core/src/components/content/LibraryCard.tsx(14 hunks)apps/core/src/utils/performance.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
apps/core/src/components/content/LibraryCard.tsx (1)
apps/core/src/lib/api-client.ts (1)
api(183-183)
apps/core/src/components/content/ContentPreviewCard.tsx (1)
apps/core/src/lib/api-client.ts (1)
api(183-183)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Performance & Load Tests
🔇 Additional comments (15)
apps/core/src/utils/performance.ts (2)
17-17: Correct fix for DEV environment check.The change from
=== "true"to=== trueis correct. In Vite,import.meta.env.DEVis a boolean, not a string. The previous code would never enable performance monitoring in development mode.
126-126: Consistent fix for DEV environment check.This change aligns with the constructor fix at line 17, ensuring performance instrumentation is correctly disabled in non-development environments.
apps/core/__tests__/integration/validation.test.ts (2)
47-55: Appropriate refactor to object-context validation.Moving from top-level optional schemas to object-context is more realistic and aligns with how TypeBox schemas are typically used in API validation. This change makes the tests more valuable.
364-373: Consistent with earlier object-context refactoring.The test now properly validates optional fields within objects, which is the expected use case in practice.
apps/core/__tests__/integration/load.test.ts (4)
18-33: Well-implemented server availability check.The beforeAll hook with a 2-second timeout and AbortController is a clean way to detect server availability. The check uses the
/api/health/readyendpoint which is appropriate for this purpose.
37-56: Good refactor to handle server unavailability.Returning
nullwhen the server is unavailable is a clean pattern that works well with the type guard helper. The early-return and logging provide clear feedback to developers.
58-63: Effective type guard pattern.The
skipIfNoServertype guard provides type-safe null checking while maintaining readability. This pattern is used consistently throughout the test file.
74-74: Consistent skip guards across all tests.All load tests now have appropriate guards that skip execution when the server is unavailable. This prevents false negatives in environments where the server isn't running (e.g., CI pipelines without backend).
Also applies to: 103-103, 129-129, 154-154, 178-178, 203-203, 223-226, 246-246, 269-269, 288-288, 307-307, 329-329, 350-350, 365-368, 389-389, 411-411, 435-435, 457-457, 482-482, 505-505
apps/core/src/components/content/LibraryCard.tsx (5)
482-487: Consistent API payload change from imageData to imageUrl.The change from
imageDatatoimageUrlusing data URL format is more explicit and aligns with web standards. The implementation correctly prepends the data URL prefix.
529-533: Consistent with banner save implementation.The portrait save follows the same pattern as banner save, using
imageUrlwith data URL format.
580-584: Upload handlers also use imageUrl consistently.The upload handlers for both portrait and banner use the same
imageUrlparameter, maintaining consistency across all image save operations.Also applies to: 635-640
437-441: The review comment is incorrect.The
ContentIteminterface definesidas a requiredstringproperty, not an optional one. The type guard at line 430 (if (item.type !== "quest") return;) narrows the union by thetypediscriminator, which does not affect the availability of the requiredidproperty. Therefore,item.idis guaranteed to be defined after the type guard, and the non-null assertion at line 439 is safe and unnecessary but not a risk.The other handlers in the file (save, delete) include similar defensive checks like
if (!item.id) return;despiteidbeing required, suggesting over-defensive coding patterns rather than indicating thatidcan actually be undefined.Likely an incorrect or invalid review comment.
292-301: The code is safe; item.id is guaranteed to exist.The
ContentIteminterface definesidas a required string property (non-optional). Sinceitemis always of typeContentItem,item.idis guaranteed to exist at all call sites. The non-null assertion (!) is redundant but not unsafe.apps/core/src/components/content/ContentPreviewCard.tsx (2)
227-231: Consistent API payload change with proper guard.The change from
imageDatatoimageUrlmatches the updates in LibraryCard. Note that line 212 already guards against missingcontent.id:if (!portraitUrl || !content.id) return;, so the non-null assertion on line 229 is safe.
90-96: The non-null assertion is safe and unnecessary.The
GeneratedContentclass definesid: stringas a required field, and thecreatefactory method ensures it's always set by defaulting tocrypto.randomUUID()if not provided. Sincecontentis typed asGeneratedContent,content.idcannot be undefined—the non-null assertion is defensive but not needed.
User description
PR Type
Bug fix, Tests, Enhancement
Description
Add server availability checks to load tests with graceful skipping
Register TypeBox format validators for email, uuid, uri, date-time
Fix Optional type tests to use object context for proper validation
Fix type mismatches: ownerId null to empty string, import.meta.env.DEV string to boolean
Add entityId parameter to NPC portrait and quest banner generation calls
Change imageData to imageUrl with data URL format in save-portrait calls
Code formatting improvements for consistency
Diagram Walkthrough
File Walkthrough
load.test.ts
Add server availability checks to load testsapps/core/tests/integration/load.test.ts
beforeAllhook to check server availability before running testsrunLoadfunction to returnnullwhen server unavailableskipIfNoServertype guard helper for conditional test executionvalidation.test.ts
Register TypeBox format validators and fix Optional testsapps/core/tests/integration/validation.test.ts
FormatRegistryfrom TypeBox for format validationprojects.ts
Fix ownerId type mismatch in project creationapps/core/server/routes/projects.ts
ownerIddefault value fromnullto empty string""performance.ts
Fix import.meta.env.DEV boolean comparisonapps/core/src/utils/performance.ts
import.meta.env.DEVcomparison from string"true"to booleantrueusePerformanceMeasurehook comparisonsContentPreviewCard.tsx
Add entityId to portrait generation and fix imageUrl formatapps/core/src/components/content/ContentPreviewCard.tsx
entityIdparameter to NPC portrait generation API callimageDatatoimageUrlwith full data URL format insave-portrait call
data:image/png;base64,prefix in imageUrl parameterLibraryCard.tsx
Add entityId to generation calls and fix imageUrl formatapps/core/src/components/content/LibraryCard.tsx
entityIdparameter to NPC portrait and quest banner generationcalls
imageDatatoimageUrlwith full data URL format in allsave-portrait calls
data:image/png;base64,prefix in imageUrl parametersstatements
Summary by CodeRabbit
New Features
Bug Fixes
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.