feat(compass-collection): Avoid sending sample values for binary fields - CLOUDP-350484 #7439

jcobis · 2025-10-09T20:04:01Z

Description

Binary fields containing large embeddings are converted to massive base64 strings in sample values, causing LLM request payloads to exceed size limits.

Solution:
Exclude sample values for Binary fields while preserving type information. The LLM can still map Binary fields appropriately using just the MongoDB type.

Also, to limit request schema size:

Changed number of sample values sent from 10 to 5
Limited long strings
Rounded probabilities

Checklist

New tests and/or benchmarks are included
Documentation is changed or added
If this change updates the UI, screenshots/videos are added and a design review is requested
I have signed the MongoDB Contributor License Agreement (https://www.mongodb.com/legal/contributor-agreement)

Types of changes

Backport Needed
Patch (non-breaking change which fixes an issue)
Minor (non-breaking change which adds functionality)
Major (fix or feature that would cause existing functionality to change)

Copilot

Pull Request Overview

This PR optimizes schema-to-field-info transformation to prevent LLM request payload size issues by excluding sample values for Binary fields (which often contain large embeddings) while preserving type information. It also implements several payload size optimizations including reducing sample value counts, truncating long strings, and rounding probabilities.

Exclude sample values for Binary fields to avoid massive base64 strings from embeddings
Reduce maximum sample values from 10 to 5 and implement string length limits
Round probabilities to 2 decimal places for cleaner output

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
transform-schema-to-field-info.ts	Implements Binary field sample value exclusion, string truncation, and probability rounding
transform-schema-to-field-info.spec.ts	Updates tests to reflect new limits and adds comprehensive test coverage for new features

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-10-09T20:32:10Z

packages/compass-collection/src/transform-schema-to-field-info.ts

  if (value instanceof Binary) {
-    return value.toString('base64');
+    // For Binary data, provide a descriptive placeholder instead of the actual data
+    // to avoid massive base64 strings that can break LLM requests
+    // (Defensive check: should never be called, since sample values for binary are skipped)
+    const sizeInBytes = value.buffer?.length || 0;
+    return `<Binary data: ${sizeInBytes} bytes>`;
  }


This defensive code in convertBSONToPrimitive is unreachable since Binary sample values are now excluded at line 307. Consider removing this code path or converting it to a warning/error if it's truly defensive.

Copilot is right about this one, no?

converting it to a warning/error if it's truly defensive.

It does seem like an invariant to me where throwing an error would be better.

Agreed! Addressing in a fast-follow PR to unblock debugging server call failures

codeowners-service-app · 2025-10-09T20:35:21Z

Assigned kraenhansen for team compass-developers because nbbeeken is out of office.

addaleax · 2025-10-10T09:30:04Z

packages/compass-collection/src/transform-schema-to-field-info.ts

  if (value instanceof ObjectId) {
    return value.toString();
  }
  if (value instanceof Binary) {


Side note: Avoid instanceof if you can and rely on _bsontype instead, instanceof is notoriously fragile in JS and will break if e.g. value and Binary come from different instances of the BSON library (like when somebody accidentally breaks package hoisting in the package lock)

like when somebody accidentally breaks package hoisting in the package lock

I suspect that will cause all sorts of other issues? Could we check for that earlier as a precondition to allow ourselves to use idiomatic JS instead? Perhaps BSON could register something in the global to warn if two instances are loaded into a program ( - in any case, certainly not for this PR to fix 🙂)

If we really want to avoid instanceof on BSON types, perhaps we should have a lint rule checking that?

We should just avoid instanceof altogether 🙂

I suspect that will cause all sorts of other issues?

It will work mostly fine, except for the part where the object identities mismatch

Could we check for that earlier as a precondition to allow ourselves to use idiomatic JS instead?

Well, I'd argue that avoiding instanceof is the idiomatic thing to do in JS – in any case, as a developer, you kind of need to be aware of the potential pitfalls when using it.

If we really want to avoid instanceof on BSON types, perhaps we should have a lint rule checking that?

Yeah, I wouldn't mind that. There don't seem to be many uses of instanceof in the codebase that couldn't easily be replaced with a better check.

We should just avoid instanceof altogether 🙂

Sent you a DM - I'd love to discuss that more 🙂

addaleax · 2025-10-10T09:31:19Z

packages/compass-collection/src/transform-schema-to-field-info.ts

  if (value instanceof Binary) {
-    return value.toString('base64');
+    // For Binary data, provide a descriptive placeholder instead of the actual data
+    // to avoid massive base64 strings that can break LLM requests
+    // (Defensive check: should never be called, since sample values for binary are skipped)
+    const sizeInBytes = value.buffer?.length || 0;
+    return `<Binary data: ${sizeInBytes} bytes>`;
  }


Copilot is right about this one, no?

ncarbon · 2025-10-10T15:05:33Z

packages/compass-collection/src/transform-schema-to-field-info.ts

+    typeof value === 'string' &&
+    value.length > MAX_STRING_SAMPLE_VALUE_LENGTH
+  ) {
+    return value.substring(0, MAX_STRING_SAMPLE_VALUE_LENGTH) + '...';


Just wondering - is it still worth it to use a truncated string?

ncarbon · 2025-10-10T15:11:52Z

packages/compass-collection/src/transform-schema-to-field-info.ts

+    // to avoid massive base64 strings that can break LLM requests
+    // (Defensive check: should never be called, since sample values for binary are skipped)
+    const sizeInBytes = value.buffer?.length || 0;
+    return `<Binary data: ${sizeInBytes} bytes>`;


I know this is unlikely to happen, but how would this be handled down the line if it ever does return this?

Removed fallback value in favor of throwing error if ever getting to this state: #7456

kraenhansen

Minor suggestions - nothing blocking a merge IMO.

Skip binary for sample values

a415ff0

github-actions bot added the feat label Oct 9, 2025

jcobis added 2 commits October 9, 2025 16:09

Comment

08f713a

WIP

6eb2fb9

jcobis added the no release notes Fix or feature not for release notes label Oct 9, 2025

jcobis changed the title ~~feat(compass-collection): Skip binary for sample values - CLOUDP-350484~~ feat(compass-collection): Avoid sending sample values for binary fields - CLOUDP-350484 Oct 9, 2025

jcobis marked this pull request as ready for review October 9, 2025 20:31

jcobis requested a review from a team as a code owner October 9, 2025 20:31

jcobis requested review from Copilot and nbbeeken October 9, 2025 20:31

Copilot AI reviewed Oct 9, 2025

View reviewed changes

codeowners-service-app bot requested a review from kraenhansen October 9, 2025 20:35

Merge branch 'main' into CLOUDP-350484

d8aa8b2

addaleax reviewed Oct 10, 2025

View reviewed changes

ncarbon reviewed Oct 10, 2025

View reviewed changes

kraenhansen approved these changes Oct 14, 2025

View reviewed changes

jcobis merged commit 54d7c77 into main Oct 14, 2025
61 checks passed

jcobis deleted the CLOUDP-350484 branch October 14, 2025 17:48

jcobis mentioned this pull request Oct 14, 2025

feat(compass-collection): Follow-ups tweaks to convertBSONToPrimitive - CLOUDP-351455 #7456

Merged

8 tasks

feat(compass-collection): Avoid sending sample values for binary fields - CLOUDP-350484 #7439

feat(compass-collection): Avoid sending sample values for binary fields - CLOUDP-350484 #7439

Conversation

jcobis commented Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Types of changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Oct 9, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kraenhansen Oct 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codeowners-service-app bot commented Oct 9, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kraenhansen Oct 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jcobis Oct 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kraenhansen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jcobis commented Oct 9, 2025 •

edited

Loading

kraenhansen Oct 14, 2025 •

edited

Loading

kraenhansen Oct 14, 2025 •

edited

Loading

jcobis Oct 14, 2025 •

edited

Loading