Skip to content

Conversation

@david-roper
Copy link
Collaborator

@david-roper david-roper commented Oct 1, 2025

Works on some expand array data fixes

turns the RecordArrayValue into a record type with a key and primitive typed data values

changed expandData to parse the record instead of each value within it

adjusted error msg

Summary by CodeRabbit

  • Refactor

    • Updated the instrument measure values schema to use arrays of keyed records instead of arrays of raw values, improving data consistency and validation. Integrations sending measure values may need to adjust payloads to the new structure.
  • Chores

    • Internal type adjustments to support the new schema structure.

@david-roper david-roper self-assigned this Oct 1, 2025
@david-roper david-roper requested a review from joshunrau as a code owner October 1, 2025 21:06
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 1, 2025

Walkthrough

Introduces an internal schema type $RecordArrayPair as a record of string keys to $RecordArrayFieldValue and updates $InstrumentMeasureValue to be an array of $RecordArrayPair instead of an array of $RecordArrayFieldValue in packages/schemas/src/instrument/instrument.base.ts.

Changes

Cohort / File(s) Summary
Schema: instrument measure value typing
packages/schemas/src/instrument/instrument.base.ts
Added internal const $RecordArrayPair = z.record(z.string(), $RecordArrayFieldValue); changed $InstrumentMeasureValue to z.array($RecordArrayPair) from z.array($RecordArrayFieldValue); no new exports introduced.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • Expand array export #1146 — Also modifies $InstrumentMeasureValue in the same file, but to z.array($RecordArrayFieldValue), indicating a conflicting or alternative approach to the same schema change.

Suggested reviewers

  • joshunrau

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title “Record array fixes” directly references the main change of updating array handling to a record‐based type and aligns with the intent to fix record array parsing, making it relevant to the changeset. It is concise and avoids unnecessary details while highlighting the core area affected.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6c9ef9e and e69c196.

📒 Files selected for processing (1)
  • packages/schemas/src/instrument/instrument.base.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/schemas/src/instrument/instrument.base.ts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 88edf61 and 6c9ef9e.

📒 Files selected for processing (2)
  • apps/api/src/instrument-records/instrument-records.service.ts (1 hunks)
  • packages/schemas/src/instrument/instrument.base.ts (1 hunks)
🔇 Additional comments (2)
packages/schemas/src/instrument/instrument.base.ts (1)

95-98: Schema change correctly implements record-of-primitives structure.

The conversion from a primitive union to z.record(z.string(), z.union([...])) aligns with the PR objective to make RecordArrayValue a record type with keys and primitive-typed values. This now properly represents array elements as objects rather than scalars.

apps/api/src/instrument-records/instrument-records.service.ts (1)

433-441: Field iteration logic is correct.

The iteration over objectEntry fields and direct use of dataValue for measureValue aligns with the PR objective to parse the record and use raw field values.

Verify this works as intended after fixing the control flow issue flagged above.

@joshunrau joshunrau merged commit fd57ed8 into DouglasNeuroInformatics:main Oct 2, 2025
1 check passed
@david-roper david-roper deleted the record-array-fixes branch November 6, 2025 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants