Skip to content

fix(api): scope segment batch deletes#38217

Open
WH-2099 wants to merge 1 commit into
mainfrom
fix/scope-segment-batch-delete
Open

fix(api): scope segment batch deletes#38217
WH-2099 wants to merge 1 commit into
mainfrom
fix/scope-segment-batch-delete

Conversation

@WH-2099

@WH-2099 WH-2099 commented Jun 30, 2026

Copy link
Copy Markdown
Member

Summary

  • scope batch segment deletes to the tenant, dataset, and document selected by the verified rows
  • use the scoped segment ids for database deletion instead of raw request ids
  • add a regression assertion for mixed owned and foreign segment ids

Root cause

PR #38177 added nested resource lookup safeguards for single segment and child chunk paths, but the batch segment delete path still deleted by the raw request id list after the scoped lookup.
A request that mixed valid ids with foreign ids could therefore delete records outside the selected document.

Follow-up to #38177 (comment).

Tests

  • env -u HTTP_PROXY -u HTTPS_PROXY -u ALL_PROXY -u http_proxy -u https_proxy -u all_proxy -u NO_PROXY -u no_proxy DEBUG=false uv run --project api pytest api/tests/unit_tests/services/test_dataset_service_segment.py -q
  • uv run --project api ruff check api/services/dataset_service.py api/tests/unit_tests/services/test_dataset_service_segment.py
  • uv run --project api ruff format --check api/services/dataset_service.py api/tests/unit_tests/services/test_dataset_service_segment.py

@WH-2099 WH-2099 self-assigned this Jun 30, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Pyrefly Type Coverage

Metric Base PR Delta
Type coverage 51.57% 51.57% 0.00%
Strict coverage 51.08% 51.08% 0.00%
Typed symbols 30,992 30,992 0
Untyped symbols 29,384 29,384 0
Modules 2935 2935 0

@github-actions

Copy link
Copy Markdown
Contributor

Pyrefly Diff

base → PR
--- /tmp/pyrefly_base.txt	2026-06-30 10:08:30.533660785 +0000
+++ /tmp/pyrefly_pr.txt	2026-06-30 10:08:15.521673888 +0000
@@ -7088,13 +7088,13 @@
 ERROR Argument `SimpleNamespace` is not assignable to parameter `dataset` with type `Dataset` in function `services.dataset_service.SegmentService.delete_child_chunk` [bad-argument-type]
    --> tests/unit_tests/services/test_dataset_service_segment.py:250:64
 ERROR Argument `SimpleNamespace` is not assignable to parameter `segment` with type `DocumentSegment` in function `services.dataset_service.SegmentService.update_child_chunk` [bad-argument-type]
-   --> tests/unit_tests/services/test_dataset_service_segment.py:913:49
+   --> tests/unit_tests/services/test_dataset_service_segment.py:922:49
 ERROR Argument `SimpleNamespace` is not assignable to parameter `document` with type `Document` in function `services.dataset_service.SegmentService.update_child_chunk` [bad-argument-type]
-   --> tests/unit_tests/services/test_dataset_service_segment.py:913:68
+   --> tests/unit_tests/services/test_dataset_service_segment.py:922:68
 ERROR Argument `SimpleNamespace` is not assignable to parameter `dataset` with type `Dataset` in function `services.dataset_service.SegmentService.update_child_chunk` [bad-argument-type]
-   --> tests/unit_tests/services/test_dataset_service_segment.py:913:87
+   --> tests/unit_tests/services/test_dataset_service_segment.py:922:87
 ERROR Argument `SimpleNamespace` is not assignable to parameter `dataset` with type `Dataset` in function `services.dataset_service.SegmentService.delete_child_chunk` [bad-argument-type]
-   --> tests/unit_tests/services/test_dataset_service_segment.py:927:60
+   --> tests/unit_tests/services/test_dataset_service_segment.py:936:60
 ERROR Argument `str` is not assignable to parameter `type` with type `Literal['basic', 'bearer', 'custom'] | None` in function `services.entities.external_knowledge_entities.external_knowledge_entities.AuthorizationConfig.__init__` [bad-argument-type]
    --> tests/unit_tests/services/test_external_dataset_service.py:118:60
 ERROR Argument `str` is not assignable to parameter `type` with type `Literal['api-key', 'no-auth']` in function `services.entities.external_knowledge_entities.external_knowledge_entities.Authorization.__init__` [bad-argument-type]

@WH-2099 WH-2099 marked this pull request as ready for review June 30, 2026 10:17
@dosubot dosubot Bot added the size:XS This PR changes 0-9 lines, ignoring generated files. label Jun 30, 2026
@WH-2099 WH-2099 enabled auto-merge June 30, 2026 10:22
@codecov

codecov Bot commented Jun 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.36%. Comparing base (44e85c0) to head (cacaa5e).

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #38217      +/-   ##
==========================================
- Coverage   85.36%   85.36%   -0.01%     
==========================================
  Files        4970     4970              
  Lines      258774   258774              
  Branches    49076    49076              
==========================================
- Hits       220893   220891       -2     
- Misses      33576    33578       +2     
  Partials     4305     4305              
Flag Coverage Δ
api 85.51% <100.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XS This PR changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant