Skip to content

fix: Use bracket notation for document body keys#1734

Merged
phact merged 1 commit into
pr-1666-filter-id-mainfrom
fix-typescript-sdk-test
Jun 2, 2026
Merged

fix: Use bracket notation for document body keys#1734
phact merged 1 commit into
pr-1666-filter-id-mainfrom
fix-typescript-sdk-test

Conversation

@edwinjosechittilappilly

Copy link
Copy Markdown
Collaborator

Change assignments to the request body to use bracket notation (body['filename'], body['filter_id']) instead of dot notation. This ensures the string keys are set explicitly on the Record<string, string> and avoids relying on declared property names.

Change assignments to the request body to use bracket notation (body['filename'], body['filter_id']) instead of dot notation. This ensures the string keys are set explicitly on the Record<string, string> and avoids relying on declared property names.
Copilot AI review requested due to automatic review settings June 2, 2026 16:26
@coderabbitai

coderabbitai Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 2a248b78-d00d-4d57-9884-76f0a66c3cbf

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-typescript-sdk-test

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.

@github-actions github-actions Bot added the bug 🔴 Something isn't working. label Jun 2, 2026

Copilot AI 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.

Pull request overview

This PR updates the TypeScript SDK’s DocumentsClient.delete() request-body construction to use bracket notation for string keys, ensuring keys like filename and filter_id are set explicitly on a Record<string, string> without relying on property access.

Changes:

  • Switch request body assignments from dot notation to bracket notation for filename and filter_id.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +169 to +170
if (opts.filename) body['filename'] = opts.filename;
if (opts.filterId) body['filter_id'] = opts.filterId;
@phact
phact merged commit 2529aa2 into pr-1666-filter-id-main Jun 2, 2026
8 checks passed
@github-actions
github-actions Bot deleted the fix-typescript-sdk-test branch June 2, 2026 16:34
edwinjosechittilappilly added a commit that referenced this pull request Jun 3, 2026
Change assignments to the request body to use bracket notation (body['filename'], body['filter_id']) instead of dot notation. This ensures the string keys are set explicitly on the Record<string, string> and avoids relying on declared property names.
edwinjosechittilappilly added a commit that referenced this pull request Jun 3, 2026
* missed filter_id fixes

* unit test

* clarify filter comments

* Use bracket notation for document body keys (#1734)

Change assignments to the request body to use bracket notation (body['filename'], body['filter_id']) instead of dot notation. This ensures the string keys are set explicitly on the Record<string, string> and avoids relying on declared property names.

* fix: Update integration.test.ts (#1737)

---------

Co-authored-by: Edwin Jose <edwin.jose@datastax.com>
edwinjosechittilappilly added a commit that referenced this pull request Jun 22, 2026
* missed filter_id fixes

* unit test

* clarify filter comments

* Use bracket notation for document body keys (#1734)

Change assignments to the request body to use bracket notation (body['filename'], body['filter_id']) instead of dot notation. This ensures the string keys are set explicitly on the Record<string, string> and avoids relying on declared property names.

* fix: Update integration.test.ts (#1737)

---------

Co-authored-by: Edwin Jose <edwin.jose@datastax.com>
(cherry picked from commit db9ae7f)
edwinjosechittilappilly added a commit that referenced this pull request Jun 26, 2026
* fix: Add filter_id support and v1 filter resolution (#1666)

* Add filter_id support and v1 filter resolution

Add server-side resolution for filter_id and extend document deletion to accept a filter_id. Introduces api.v1._filter_resolution to normalize a filter_id into concrete filters/limits/score_thresholds and to strip wildcards. v1 endpoints (chat, search, documents) now accept filter_id and merge resolved values with inline overrides; documents DELETE can delete all filenames referenced by a filter_id (with wildcard/empty data_sources rejected to avoid mass deletion). SDK updates (Python and TypeScript) allow DocumentsClient.delete to accept either filename or filter_id (mutually exclusive), include additional response fields (filenames, filter_id, per_file), and preserve idempotent semantics for filename deletes while surfacing filter-not-found errors. Tests updated/added to cover filter_id behavior, inline overrides, streaming, and validation. Additional minor logging/validation and type updates included.

* style: ruff autofix (auto)

* Clarify /v1/documents DELETE description

Expand the DELETE /v1/documents component description to indicate it can delete single or multiple documents and requires exactly one of `filename` or `filter_id`. Also notes that wildcards are rejected for safety, improving API documentation and removing ambiguity about deletion semantics.

* style: ruff autofix (auto)

* fix lint

* style: ruff autofix (auto)

* Set auth context for API-key search requests

Ensure API-key authenticated searches set the auth context and pass the user's JWT to the search service so downstream tools can resolve the user. Adds import for set_auth_context, calls it when handling API-key requests (mirroring v1 chat), and passes user.jwt_token to search_service.search instead of None. Also cleans up an unused typing import.

* Exclude ingest route; expose /v1 GETs as tools

Clarify search component docs to document `filter_id` and inline `filters` (inline filters override per-field). Document that /v1/documents/ingest is intentionally not customized/exposed via MCP because multipart/form-data uploads are not supported by FastMCP's from_fastapi conversion; clients should use the HTTP API or SDK to ingest. Add a RouteMap to explicitly exclude POST /v1/documents/ingest and consolidate route maps to expose all /v1/ routes (including GET) as MCP tools, with a note explaining that GETs are exposed as tools to better support LLM agent workflows.

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
(cherry picked from commit eda6aaf)

* fix: more filter_id issues (#1730)

* missed filter_id fixes

* unit test

* clarify filter comments

* Use bracket notation for document body keys (#1734)

Change assignments to the request body to use bracket notation (body['filename'], body['filter_id']) instead of dot notation. This ensures the string keys are set explicitly on the Record<string, string> and avoids relying on declared property names.

* fix: Update integration.test.ts (#1737)

---------

Co-authored-by: Edwin Jose <edwin.jose@datastax.com>
(cherry picked from commit db9ae7f)

* Update documents.ts

* test: raise TS SDK filter-scope test timeouts and parallelize ingests

The "filterId in chat actually scopes retrieval to data_sources" integration
test was timing out at 60s and failing CI on every run. The chat itself works
(backend logs show the response generated in ~7.6s); the 60s budget was consumed
by two sequential Docling ingests (~25s each in CI) plus the filter create before
the chat even started, so the test crossed the deadline.

Bump the three ingest-heavy filter-scope tests (chat, search, delete-by-filter)
to a 180s per-test timeout and ingest the two docs concurrently via Promise.all
to shorten the critical path. Raise the global vitest testTimeout 60s -> 120s so
lighter tests have headroom too. The Python equivalent already passes because
pytest imposes no per-test cap; no backend/SDK behavior changes.

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: Sebastián Estévez <estevezsebastian@gmail.com>
rodageve pushed a commit that referenced this pull request Jun 29, 2026
* fix: Add filter_id support and v1 filter resolution (#1666)

* Add filter_id support and v1 filter resolution

Add server-side resolution for filter_id and extend document deletion to accept a filter_id. Introduces api.v1._filter_resolution to normalize a filter_id into concrete filters/limits/score_thresholds and to strip wildcards. v1 endpoints (chat, search, documents) now accept filter_id and merge resolved values with inline overrides; documents DELETE can delete all filenames referenced by a filter_id (with wildcard/empty data_sources rejected to avoid mass deletion). SDK updates (Python and TypeScript) allow DocumentsClient.delete to accept either filename or filter_id (mutually exclusive), include additional response fields (filenames, filter_id, per_file), and preserve idempotent semantics for filename deletes while surfacing filter-not-found errors. Tests updated/added to cover filter_id behavior, inline overrides, streaming, and validation. Additional minor logging/validation and type updates included.

* style: ruff autofix (auto)

* Clarify /v1/documents DELETE description

Expand the DELETE /v1/documents component description to indicate it can delete single or multiple documents and requires exactly one of `filename` or `filter_id`. Also notes that wildcards are rejected for safety, improving API documentation and removing ambiguity about deletion semantics.

* style: ruff autofix (auto)

* fix lint

* style: ruff autofix (auto)

* Set auth context for API-key search requests

Ensure API-key authenticated searches set the auth context and pass the user's JWT to the search service so downstream tools can resolve the user. Adds import for set_auth_context, calls it when handling API-key requests (mirroring v1 chat), and passes user.jwt_token to search_service.search instead of None. Also cleans up an unused typing import.

* Exclude ingest route; expose /v1 GETs as tools

Clarify search component docs to document `filter_id` and inline `filters` (inline filters override per-field). Document that /v1/documents/ingest is intentionally not customized/exposed via MCP because multipart/form-data uploads are not supported by FastMCP's from_fastapi conversion; clients should use the HTTP API or SDK to ingest. Add a RouteMap to explicitly exclude POST /v1/documents/ingest and consolidate route maps to expose all /v1/ routes (including GET) as MCP tools, with a note explaining that GETs are exposed as tools to better support LLM agent workflows.

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
(cherry picked from commit eda6aaf)

* fix: more filter_id issues (#1730)

* missed filter_id fixes

* unit test

* clarify filter comments

* Use bracket notation for document body keys (#1734)

Change assignments to the request body to use bracket notation (body['filename'], body['filter_id']) instead of dot notation. This ensures the string keys are set explicitly on the Record<string, string> and avoids relying on declared property names.

* fix: Update integration.test.ts (#1737)

---------

Co-authored-by: Edwin Jose <edwin.jose@datastax.com>
(cherry picked from commit db9ae7f)

* Update documents.ts

* test: raise TS SDK filter-scope test timeouts and parallelize ingests

The "filterId in chat actually scopes retrieval to data_sources" integration
test was timing out at 60s and failing CI on every run. The chat itself works
(backend logs show the response generated in ~7.6s); the 60s budget was consumed
by two sequential Docling ingests (~25s each in CI) plus the filter create before
the chat even started, so the test crossed the deadline.

Bump the three ingest-heavy filter-scope tests (chat, search, delete-by-filter)
to a 180s per-test timeout and ingest the two docs concurrently via Promise.all
to shorten the critical path. Raise the global vitest testTimeout 60s -> 120s so
lighter tests have headroom too. The Python equivalent already passes because
pytest imposes no per-test cap; no backend/SDK behavior changes.

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Co-authored-by: Sebastián Estévez <estevezsebastian@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug 🔴 Something isn't working.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants