refactor(database): linear-time `mergeMultiJoin` (plus type hints) by jaskfla · Pull Request #6631 · beyondessential/tupaia

jaskfla · 2026-02-05T05:04:11Z

🦸 Review Hero

Run Review Hero
Auto-fix review suggestions
Auto-fix CI failures

gemini-code-assist · 2026-02-05T05:04:31Z

Summary of Changes

Hello @jaskfla, I'm Gemini Code Assist¹! 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 optimizing the mergeMultiJoin utility for better performance and enhancing the overall type safety and consistency across the codebase. It introduces linear-time complexity for a critical database operation and improves developer experience through better type hinting and standardized constant management.

Highlights

Performance Optimization: The mergeMultiJoin utility function has been refactored to improve its time complexity from quadratic (O(N*M)) to linear (O(N+M)) by utilizing a Set for efficient lookup of existing join tables. This significantly enhances performance when merging large arrays of join configurations.
Type Safety and Documentation: Comprehensive JSDoc type hints have been added to the mergeMultiJoin function, including @typedef for MultiJoinItem and @template, @param, @returns for the function itself. This improves code readability, maintainability, and enables better static analysis.
TypeScript Consistency: Type definitions for SingleSurveyResponseRequest and SurveyResponsesRequest in web server routes have been updated from type aliases to interface declarations, aligning with common TypeScript practices for object type extensibility.
Constant Centralization: Constants like BES_ADMIN_PERMISSION_GROUP and SURVEY_RESPONSE_DEFAULT_FIELDS have been moved from local definitions to imports from @tupaia/constants, promoting a single source of truth and reducing duplication.
Query Hook Refinement: The useSurveyResponses query hook has been refined to use a consistent params object for its query key and API call, and as const assertion was added to DEFAULT_FIELDS for stronger type inference.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog

packages/database/src/core/permissions/assertSurveyResponsePermission.js
- Refactored SQL query string formatting for improved readability without functional changes.
packages/database/src/core/utilities/mergeMultiJoin.js
- Implemented a performance optimization for mergeMultiJoin by replacing Array.prototype.find with a Set for checking unique joins, reducing time complexity from quadratic to linear.
- Added extensive JSDoc type definitions for MultiJoinItem and the mergeMultiJoin function, including @typedef, @template, @param, and @returns.
packages/datatrak-web-server/src/routes/SingleSurveyResponseRoute.ts
- Converted SingleSurveyResponseRequest from a type alias to an interface for better TypeScript extensibility.
- Centralized constants BES_ADMIN_PERMISSION_GROUP and SURVEY_RESPONSE_DEFAULT_FIELDS by importing them from @tupaia/constants.
packages/datatrak-web-server/src/routes/SurveyResponsesRoute.ts
- Converted SurveyResponsesRequest from a type alias to an interface for better TypeScript extensibility.
- Added as const assertion to the DEFAULT_FIELDS array for stronger type inference and immutability.
packages/datatrak-web/src/api/queries/useSurveyResponses.ts
- Refactored useQuery key and parameter passing to use a consistent params object.
- Updated the useQuery callback to use async/await syntax and added explicit type assertion for the return type.
- Changed boolean conversion for the enabled option from !! to Boolean() for stylistic consistency.

Activity

No specific review comments or activity have been recorded for this pull request yet.

Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.}

gemini-code-assist

Code Review

This pull request introduces a significant performance improvement to the mergeMultiJoin utility by refactoring it to a linear-time algorithm. This is an excellent optimization that will improve performance where this utility is used. The pull request also includes several other valuable refactorings, such as adding JSDoc type hints for better developer experience, centralizing constants for improved consistency, and strengthening TypeScript types with interface and as const for better type safety. The changes are well-implemented and improve the overall quality of the codebase. I have no further comments.

jaskfla · 2026-02-17T01:59:07Z

-    return baseMultiJoin;
-  }
-  const checkJoinIsUnique = join => !baseMultiJoin.find(j => j.joinWith === join.joinWith);
-  return [...baseMultiJoin].concat(...multiJoinToMerge.filter(checkJoinIsUnique));


Array.prototype.concat already returns a new array without modifying either of the originals

Both these spreads are redundant

jaskfla · 2026-02-17T02:01:55Z

+  const existingJoinTables = new Set(base.map(j => j.joinWith));
+  const uniques = source.filter(j => !existingJoinTables.has(j.joinWith));
+
+  return [...base, ...uniques];


Alternatively:

Suggested change

return [...base, ...uniques];

return base.concat(uniques);

review-hero · 2026-03-15T21:48:19Z

🦸 Review Hero Summary
2 agents reviewed this PR | 0 critical | 0 suggestions | 1 nitpick

Nitpicks

File	Line	Agent	Comment
`packages/database/src/core/utilities/mergeMultiJoin.js`	24	Performance	When all source items are duplicates, `uniques` will be empty but `[...base, ...uniques]` still allocates a new array. Adding `if (uniques.length === 0) return base;` before the spread avoids this unnecessary allocation in the common hot-path where source joins already exist.

Local fix prompt (copy to your coding agent)

Fix these issues identified on the pull request. One commit per issue fixed.

packages/database/src/core/utilities/mergeMultiJoin.js:24: When all source items are duplicates, uniques will be empty but [...base, ...uniques] still allocates a new array. Adding if (uniques.length === 0) return base; before the spread avoids this unnecessary allocation in the common hot-path where source joins already exist.

jaskfla · 2026-03-30T02:11:26Z

When all source items are duplicates, uniques will be empty but [...base, ...uniques] still allocates a new array. Adding if (uniques.length === 0) return base; before the spread avoids this unnecessary allocation in the common hot-path where source joins already exist.

Ignoring this performance nitpick because it feels wrong for it to only sometimes return a new array.

jaskfla added the cooldown label Feb 5, 2026

cursor Bot reviewed Feb 5, 2026

View reviewed changes

Comment thread packages/datatrak-web/src/api/queries/useSurveyResponses.ts Outdated

gemini-code-assist Bot reviewed Feb 5, 2026

View reviewed changes

jaskfla force-pushed the mmj branch from 476a5f1 to 3c67507 Compare February 5, 2026 06:01

jaskfla commented Feb 17, 2026

View reviewed changes

jaskfla force-pushed the mmj branch 2 times, most recently from 0d91e57 to 0c611da Compare February 24, 2026 01:36

Base automatically changed from rn-1717-test-four to rn-1545-epic-datatrak-offline March 2, 2026 04:44

Base automatically changed from rn-1545-epic-datatrak-offline to dev March 2, 2026 05:46

jaskfla force-pushed the mmj branch 2 times, most recently from f9f07bf to 7bea986 Compare March 15, 2026 21:02

jaskfla force-pushed the mmj branch 2 times, most recently from 8012274 to 28d59e7 Compare March 30, 2026 00:34

jaskfla force-pushed the mmj branch from 28d59e7 to 93dee1e Compare March 30, 2026 02:11

jaskfla force-pushed the mmj branch 2 times, most recently from 4b96503 to 5d9345b Compare April 14, 2026 02:41

O(n)

b3b4927

jaskfla force-pushed the mmj branch from 5d9345b to b3b4927 Compare April 24, 2026 01:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(database): linear-time `mergeMultiJoin` (plus type hints)#6631

refactor(database): linear-time `mergeMultiJoin` (plus type hints)#6631
jaskfla wants to merge 1 commit intodevfrom
mmj

jaskfla commented Feb 5, 2026 •

edited by github-actions Bot

Loading

Uh oh!

gemini-code-assist Bot commented Feb 5, 2026

Uh oh!

cursor Bot left a comment

Uh oh!

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

jaskfla Feb 17, 2026

Uh oh!

jaskfla Feb 17, 2026

Uh oh!

review-hero Bot commented Mar 15, 2026

Uh oh!

jaskfla commented Mar 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

jaskfla commented Feb 5, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦸 Review Hero

Uh oh!

gemini-code-assist Bot commented Feb 5, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

jaskfla Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

jaskfla Feb 17, 2026

Choose a reason for hiding this comment

Uh oh!

review-hero Bot commented Mar 15, 2026

Nitpicks

Uh oh!

jaskfla commented Mar 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

jaskfla commented Feb 5, 2026 •

edited by github-actions Bot

Loading