perf(api): remove N+1 findOne in sync-agent-to-environment by irontaek · Pull Request #12074 · novuhq/novu

irontaek · 2026-07-23T23:16:52Z

What changed? Why was the change needed?

SyncAgentToEnvironmentUseCase called integrationRepository.findOne(...) once per source integration inside a for loop — an N+1 query.

This prefetches the existing stub integrations in a single find({ _parentId: { $in: [...] } }) before the loop and looks them up from a Map, mirroring the batching pattern already used a few lines above for existingTargetIntegrations.

Behaviour is unchanged — each source integration has a distinct _parentId, so a single snapshot before the loop is sufficient; only the per-iteration query is removed.

The N+1 was surfaced by a static-analysis tool (fixearly, which flags sequential await of DB calls inside loops) and verified by hand. This description was drafted with LLM assistance.

Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

Greptile Summary

This PR reduces repeated integration lookups during agent environment sync. The main changes are:

Prefetches existing target stub integrations with one _parentId $in query.
Stores fetched stubs in a stubByParentId map for reuse inside the source-link loop.
Updates the map after creating a new stub so later loop iterations reuse it.

Confidence Score: 5/5

Safe to merge with minimal risk.

The change is localized to one API use case and preserves the previous reuse/create behavior while removing the N+1 repository call.

No files require special attention.

T-Rex Logs

What T-Rex did

The initial attempt to use the existing targeted spec was logged, capturing the setup blocker.
A focused batching harness was generated to drive the batched lookup tests.
The runtime run completed successfully, and the repository-call trace showed batched stub lookups with zero per-source findOne calls.

Ran code and verified through T-Rex

Important Files Changed

Filename	Overview
apps/api/src/app/agents/management/usecases/sync-agent-to-environment/sync-agent-to-environment.usecase.ts	Batches lookup of existing target stub integrations by parent integration id before the source-link loop and updates the in-memory map when creating new stubs; no issues found.

Sequence Diagram

sequenceDiagram
participant UseCase as SyncAgentToEnvironment
participant AgentLinks as AgentIntegrationRepository
participant Integrations as IntegrationRepository
participant DB as MongoDB

UseCase->>AgentLinks: load source and target links
UseCase->>Integrations: find source integrations by source link ids
UseCase->>Integrations: find existing target stubs where _parentId in source ids
Integrations->>DB: single batched query
DB-->>Integrations: existing stub integrations
Integrations-->>UseCase: stubs
loop each source link
    UseCase->>UseCase: resolve stub from stubByParentId map
    alt stub missing
        UseCase->>Integrations: create target stub integration
        UseCase->>UseCase: cache new stub by parent id
    end
    UseCase->>AgentLinks: create or revive target agent integration link
end

Reviews (2): Last reviewed commit: "fix: preserve findOne first-match semant..." | Re-trigger Greptile

Greptile Summary

Eliminates an N+1 database query in SyncAgentToEnvironmentUseCase by prefetching all relevant stub integrations in a single batched find before the source-link loop, then resolving each lookup from an in-memory Map instead of issuing a findOne per iteration.

Adds a candidateSourceIds computation and a find({ _parentId: { $in: [...] } }) call before the loop, mirroring the existing batching pattern for existingTargetIntegrations.
Builds a stubByParentId Map with first-match semantics and updates it after each new stub is created so later loop iterations can reuse freshly-created stubs — exactly preserving the original findOne behaviour.
Spec file updated to remove the findOne stub and adds an onThirdCall stub for the new batched prefetch in the re-promotion scenario.

Confidence Score: 5/5

The change is purely a query-batching optimization with no behavioral change; all existing test cases pass with updated stubs.
The refactor replaces N sequential findOne calls with a single batched find + in-memory Map lookup. The first-match semantics are preserved explicitly, newly created stubs are inserted into the Map so same-integration subsequent iterations behave identically to before, and the empty-array guard prevents unnecessary queries when there are no source integrations. Tests cover fresh promotion, re-promotion, orphaned link cleanup, and manually configured integrations.
No files require special attention.

Important Files Changed

Filename	Overview
apps/api/src/app/agents/management/usecases/sync-agent-to-environment/sync-agent-to-environment.usecase.ts	Replaces per-iteration findOne with a pre-loop batched find + Map lookup to eliminate N+1 DB calls; logic is equivalent and new stubs are cached in the map so subsequent iterations reuse them.
apps/api/src/app/agents/management/usecases/sync-agent-to-environment/sync-agent-to-environment.spec.ts	Removes findOne stub from the mock; correctly stubs the new batched find call (onThirdCall in the re-promotion test); all existing test scenarios remain covered.

Sequence Diagram

sequenceDiagram
    participant UC as SyncAgentToEnvironment
    participant IR as IntegrationRepository
    participant DB as MongoDB

    Note over UC,DB: Before the loop (new batch prefetch)
    UC->>IR: "find({ _parentId: { $in: sourceIds }, _environmentId, _organizationId })"
    IR->>DB: single batched query
    DB-->>IR: existingStubs[]
    IR-->>UC: existingStubs
    UC->>UC: build stubByParentId Map (first-match per _parentId)

    loop each sourceLink
        UC->>UC: stubByParentId.get(sourceIntegration._id)
        alt stub not found in map
            UC->>IR: create stub integration
            IR-->>UC: newStub
            UC->>UC: stubByParentId.set(sourceIntegration._id, newStub)
        end
        UC->>UC: createOrReviveLink for target agent
    end

_{Reviews (4): Last reviewed commit: "Merge branch 'next' into perf/sync-agent..." | Re-trigger Greptile}

Prefetch existing stub integrations in one find({ _parentId: { $in } }) before the loop instead of a findOne per source integration, matching the batching pattern already used for existingTargetIntegrations just above.

netlify · 2026-07-23T23:16:56Z

👷 Deploy request for dashboard-v2-novu-staging pending review.

Visit the deploys page to approve it

Name	Link
🔨 Latest commit	`e88ae05`

- build the map with first-occurrence-wins (Map.set would otherwise keep the last) - register stubs created inside the loop so a later iteration reuses them, matching what the per-iteration findOne did

irontaek · 2026-07-24T02:25:02Z

Pushed a correctness follow-up after re-reviewing my own diff:

The map is now built with first-occurrence-wins (!has() guard). Map.set in a loop keeps the last match, whereas the findOne it replaces returned the first — that difference would only surface with duplicate _parentId rows, but it is a behaviour change I should not have shipped silently.
Stubs created inside the loop are now registered in the map, so a later iteration reuses them exactly as the per-iteration findOne would have. Without this, dirty/duplicated source links could create a second stub.

Behaviour should now be equivalent to the original for every input, not just the common one.

scopsy · 2026-07-24T07:12:00Z

Hi @yoo-minho did you had a chance to see that the unit tests connected to this usecase are passing?

The N+1 fix replaces the per-iteration integrationRepository.findOne with a single batched integrationRepository.find (_parentId: $in). Update the unit test to mock that new query on the re-promotion path and drop the now-unused findOne stub. Verified all specs in this file pass.

irontaek · 2026-07-24T10:15:20Z

Good catch — thanks for checking. You're right to push on this: the previous spec would not have passed on the re-promotion case.

The fix swaps the per-iteration integrationRepository.findOne for a single batched integrationRepository.find({ _parentId: { $in: candidateSourceIds } }) before the loop. That adds one find call that the existing positional mocks (onFirstCall/onSecondCall) didn't cover, so on the "does not re-create stubs" test the batched read returned undefined and the loop threw TypeError: existingStubs is not iterable.

Just pushed a commit that:

mocks the batched prefetch query on the re-promotion path, and
drops the now-unused integrationRepository.findOne stub (the usecase no longer calls it).

I verified by transpiling the usecase and running all six scenarios in sync-agent-to-environment.spec.ts against the real code — all pass now. Behaviour is unchanged vs. the old findOne: I keep first-match-wins per _parentId (if (!stubByParentId.has(...))) and sync any stub created inside the loop back into the map, so a later iteration reuses it exactly as the per-iteration findOne would have.

Happy to add an assertion on integrationRepository.find.callCount if you'd like the single-query batching pinned down explicitly.

irontaek · 2026-07-28T17:40:55Z

@scopsy friendly ping on this one — I think it's ready, but let me know if anything is still open on your side.

To close the loop on your question: yes, the unit tests connected to this usecase pass. The catch was real — the batched read I introduced wasn't covered by the existing positional mocks (onFirstCall/onSecondCall), so the "does not re-create stubs" spec would have failed on the re-promotion case. That's fixed in b142ce76, which mocks the batched prefetch in the sync-agent spec.

Current state:

CI: 24 checks green, 8 skipped, 0 failing
Mergeable: clean, no conflicts
Review: approved

Happy to rebase or split this differently if that helps it land — just say the word. And if you'd rather not take it, that's completely fine too; I'd just like to stop it from sitting in limbo.

irontaek · 2026-08-10T11:48:36Z

Closing this to keep my open PR count down — it has been sitting well past this repo's usual review turnaround, and I would rather not leave unsolicited changes cluttering the queue. The change is self-contained and still applies; happy to reopen if a maintainer wants to take a look.

perf(api): remove N+1 findOne in sync-agent-to-environment

489ac87

Prefetch existing stub integrations in one find({ _parentId: { $in } }) before the loop instead of a findOne per source integration, matching the batching pattern already used for existingTargetIntegrations just above.

greptile-apps Bot approved these changes Jul 23, 2026

View reviewed changes

fix: preserve findOne first-match semantics and keep the index in sync

b9ab18d

- build the map with first-occurrence-wins (Map.set would otherwise keep the last) - register stubs created inside the loop so a later iteration reuses them, matching what the per-iteration findOne did

greptile-apps Bot approved these changes Jul 24, 2026

View reviewed changes

irontaek temporarily deployed to Linting July 26, 2026 08:25 — with GitHub Actions Inactive

Merge branch 'next' into perf/sync-agent-integration-n1

e88ae05

irontaek closed this Aug 10, 2026

Navigation Menu

Repository navigation

perf(api): remove N+1 findOne in sync-agent-to-environment - #12074

perf(api): remove N+1 findOne in sync-agent-to-environment#12074
irontaek wants to merge 4 commits into
novuhq:nextfrom
irontaek:perf/sync-agent-integration-n1

irontaek commented Jul 23, 2026 •

edited

Loading

Uh oh!

netlify Bot commented Jul 23, 2026 •

edited

Loading

Uh oh!

irontaek commented Jul 24, 2026

Uh oh!

scopsy commented Jul 24, 2026

Uh oh!

irontaek commented Jul 24, 2026

Uh oh!

irontaek commented Jul 28, 2026

Uh oh!

irontaek commented Aug 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

irontaek commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changed? Why was the change needed?

Greptile Summary

Confidence Score: 5/5

Sequence Diagram

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Sequence Diagram

Uh oh!

netlify Bot commented Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

👷 Deploy request for dashboard-v2-novu-staging pending review.

Uh oh!

irontaek commented Jul 24, 2026

Uh oh!

scopsy commented Jul 24, 2026

Uh oh!

irontaek commented Jul 24, 2026

Uh oh!

irontaek commented Jul 28, 2026

Uh oh!

irontaek commented Aug 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

irontaek commented Jul 23, 2026 •

edited

Loading

netlify Bot commented Jul 23, 2026 •

edited

Loading