26 refactor health record schema to support unified structure with parent child relationship#47
Conversation
…ion logic accordingly
…nd streamline conversation creation logic
… update logic for health records
…perator and configured .findByIdAndUpdate operation to run validators
… adopt that pattern. Remove logs.
…ake it optional and adjust validation checks for symptoms, treatments tried, and medical consultations in customValidators
…e route and clean up validation in customValidators
… conversation history management
…ry so crucial instrunctions for outputing json object are preserved
…ment and validation handling
…on dates to Date objects
…e helper function for date processing. Remove unused type from healthRecordValidation
…routes to ensure proper validation
AladinJmila
left a comment
There was a problem hiding this comment.
Nice progress, we're moving in the right direction 👌
| default: "me", | ||
| //required: true, | ||
| }, | ||
| rootId: { |
There was a problem hiding this comment.
could we rename this to parentRecordId? rootId could mean anything and can be ambiguous to people that are not familiar with the code or the consumers of our API.
| } | ||
| next(); | ||
| }); | ||
|
|
There was a problem hiding this comment.
I don’t think we need this hook, since our logic should already prevent children from having an updates array. The schema validator is already safeguarding against this, and we should also have additional checks in our endpoints.
In my opinion, the hook is not a good solution for two reasons:
- If, for whatever reason, data does end up in the updates array, it will be silently cleared and lost without us noticing.
- This silent data removal could hide a bug in our code that we actually need to fix. The validator is better because it throws an error and makes the issue visible.
| .optional() | ||
| .default([]), | ||
| updates: z.array(Z_HealthRecordUpdate).optional().default([]), | ||
| updates: z.array(z.string().regex(/^[0-9a-fA-F]{24}$/, "Invalid ObjectId format")).optional(), |
There was a problem hiding this comment.
There is a built-in validator for object ids in mongoose since this is a common check. This can look like this:
updates: z.array(Z_ObjectId).optional(),Where Z_ObjectId is:
const Z_ObjectId = z.string().refine((val) => Types.ObjectId.isValid(val), {
message: "Invalid ObjectId format",
});You then reuse to validate the parentRecordId as well
parentRecordId: Z_ObjectId.optional().nullable(),| rootId: true, // rootId is immutable | ||
| updates: true, // updates array is managed by the system | ||
| }) | ||
| .partial(); |
There was a problem hiding this comment.
This seemed to be off to me. According to AI, this should be the right implementation, unless you had other reasons for it:
// For HTTP PATCH: Any field can be updated, all are optional, but when provided must be valid
export const Z_HealthRecordPatch = Z_HealthRecord.omit({
rootId: true, // Immutable
updates: true, // System-managed
}).partial();| symptoms: true, | ||
| status: true, | ||
| treatmentsTried: true, | ||
| medicalConsultations: true, |
There was a problem hiding this comment.
Not sure why we need this any longer. Can't we just use the new Z_HealthRecordPatch instead for all our update operations?
| export type HealthRecordType = z.infer<typeof Z_HealthRecord> & { | ||
| parentId?: Types.ObjectId | null; | ||
| updates?: Types.ObjectId[]; | ||
| }; |
There was a problem hiding this comment.
This should be, the below, since we need to remove the old entries and replace them with the new ones:
export type HealthRecordType = Omit<z.infer<typeof Z_HealthRecord>, 'parentRecordId' | 'updates'> & {
parentRecordId?: Types.ObjectId | null;
updates?: Types.ObjectId[];
};| if (!conversation) return; | ||
|
|
||
| conversation.history = conversation.history.filter((prompt) => prompt.role === "user"); | ||
| conversation.history = conversation.history.filter((prompt) => prompt.role === "user" || prompt.role === "system"); |
There was a problem hiding this comment.
Why did you add system here as well? I was focusing only user input because they are more concise and should have the raw data which is captured by the AI. system prompts are lengthy and would cost us a lot of tokens, we also should not care about old system prompts, we only need to worry about the current one.
Summary
rootIdfield (null for top-level records) andupdatesarray to track child recordsupdatesarray/updates/:healthRecordIdfor true partial updates (form edits and conversational corrections)/updates/:parentId) for creating chronological health record snapshotsZ_HealthRecordPatchschema and extracted date pre-processing logic into reusablepreprocessDates()helperChecklist
mainbranch.