Skip to content

v0.5.1 - #168

Merged
wellis3 merged 2 commits into
deployedfrom
main
Jun 29, 2026
Merged

v0.5.1#168
wellis3 merged 2 commits into
deployedfrom
main

Conversation

@wfelliss

Copy link
Copy Markdown
Owner

v0.5.1

wfelliss and others added 2 commits June 29, 2026 15:15
fix: changed upper bound to be able to be null so it can be set blank…
@wfelliss
wfelliss requested a review from wellis3 June 29, 2026 14:18
@wellis3
wellis3 merged commit 8d2332d into deployed Jun 29, 2026
4 checks passed

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

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 removes the ".notNull()" constraint from the "upper_bound_idx" column in the "runs" database schema. The reviewer recommends keeping the constraint to prevent data integrity issues and maintain type safety, noting that the service layer already handles optional inputs appropriately.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

profile: integer("profile").references(() => profiles.id), // associated profile
lower_bound_idx: integer("lower_bound_idx").notNull().default(0), // inclusive trim start index (default: 0)
upper_bound_idx: integer("upper_bound_idx").notNull(), // inclusive trim end index (set by service layer to length-1)
upper_bound_idx: integer("upper_bound_idx"), // inclusive trim end index (set by service layer to length-1)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Description

Removing the .notNull() constraint from upper_bound_idx introduces potential data integrity issues and type safety concerns across the codebase.

Why this is an issue:

  1. Data Integrity: Every run should have a valid upper bound (defaulting to length - 1). Removing .notNull() allows runs to be persisted with a null upper bound, which violates the core business logic.
  2. Type Safety: This change alters the inferred TypeScript type of Run["upper_bound_idx"] from number to number | null. Any existing frontend or backend code (such as resolveTrimBounds or graphing components) that expects a number will now need to handle null, increasing the risk of runtime errors or requiring widespread null-coalescing checks.
  3. Redundancy: The service layer (runs.service.ts) already gracefully handles optional inputs by resolving upper_bound_idx to Math.max(data.length - 1, 0) if not provided during creation or updates. Therefore, making the database column nullable is not required to support optional inputs at the API level.

Recommendation

Keep the .notNull() constraint to ensure database integrity and maintain strict type safety.

Suggested change
upper_bound_idx: integer("upper_bound_idx"), // inclusive trim end index (set by service layer to length-1)
upper_bound_idx: integer("upper_bound_idx").notNull(), // inclusive trim end index (set by service layer to length-1)

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