Skip to content

fix(knowledge): reconcile segment indexes on repeated status requests - #45239

Open
Zhi-Xiang-Guo wants to merge 2 commits into
langgenius:mainfrom
Zhi-Xiang-Guo:fix/segment-status-dispatch-recovery
Open

Zhi-Xiang-Guo wants to merge 2 commits into
langgenius:mainfrom
Zhi-Xiang-Guo:fix/segment-status-dispatch-recovery

Conversation

@Zhi-Xiang-Guo

@Zhi-Xiang-Guo Zhi-Xiang-Guo commented Nov 1, 2026 •

Copy link
Copy Markdown

Description

A Console segment enable/disable request commits its status before Celery publication. After a broker failure, a retry previously skipped the segment because its persisted status already matched, leaving the index unchanged.

Dispatch reconciliation for all eligible explicitly requested segments, including those whose desired status is already persisted. Update status fields only for actual transitions, preserving the original disabled_at/disabled_by on retries. Keep document/segment indexing guards and tenant/resource scoping.

Related Issues

Fixes #45238. Related #43865 covers a different enabled-only Service API path.

Module(s) Affected

  • api/services/knowledge/segments/application.py
  • api/tests/unit_tests/services/knowledge/segments/test_adapters.py

Testing

Four failure/retry cases cover enable and disable for both single-segment and mixed batches (already at the target state plus a segment requiring a transition). All four fail on the original baseline and pass with the fix. The mixed batch retries must dispatch both eligible segments, exclude foreign-tenant segments, and preserve existing disable audit metadata. Three additional cases exercise repeated batch-enable tasks for disabled, archived, and incomplete documents; the existing non-COMPLETED-segment test also passes. Expanded service/adapter/worker suite: 144 passed, 6 warnings. Real API + worker fault injection restored keyword retrieval from 0 to 1; symmetric disable retry removed stale keywords. Test assertions also preserve disable audit metadata and exclude foreign-tenant segments.

  • Baseline: main@4c521a3c0a1f60607c8a7bcf90792f34adcf18d0; branch fix/segment-status-dispatch-recovery; commit 56e35fee231b6fc46dc3c2a544e9af06adbbba2b.
  • Actual deployment: isolated source API + Celery, PostgreSQL 15 and Redis 6; macOS arm64, Python 3.13.15.
  • Local unit fixtures use real SQLite repositories and existing VectorProbe/model stubs; these are distinct from the real-service API validation.
  • Ruff check/format, targeted Pyrefly and git diff --check passed. Import linter: 78 contracts kept, 0 broken. Response contract lint: 762 valid, 0 mismatch, 378 unknown (not a claim of full coverage).
  • Relevant repository diff guards passed. Full monorepo test/lint was not run. Remote workflows require approval or remain pending; CI has not passed. Independent human code review is pending.

Latest test-only update: local ba500552a6e323adb56de3462549f51a32a07e32, remote ed365091de039abd7c4cf3830e9c56a435044f82 (identical source trees). Production logic is unchanged; the real application before/after evidence above comes from the original implementation validation, not a new deployment run.

Validation for this update: uv run --project api --no-sync pytest -o addopts="" api/tests/unit_tests/services/knowledge/segments/test_adapters.py api/tests/unit_tests/services/knowledge/segments/test_application.py api/tests/unit_tests/tasks/test_enable_segments_to_index_task.py -q — 144 passed. Ruff check/format, the no-new-test-doubles guard, and git diff --check passed. Targeted Pyrefly reports one existing contextmanager/Iterator deprecation warning in the worker test helper; the unchanged pre-update files produce the same warning. No new type diagnostics.

Checklist

  • Targets main; applicable contribution and architecture guidance reviewed.
  • Regression cases and real application before/after evidence prepared.
  • No API payload or schema migration required.
  • Full monorepo checks / remote CI completed.
  • Publication authorized by the author on 2026-11-01 after agent review.
  • Independent human code review completed.

Additional Notes

AI-assisted implementation and validation. This is explicit retry recovery, not an Outbox: it does not autonomously retry a lost dispatch, resolve opposite-action ordering, or guarantee exactly-once delivery. Repeated successful requests can enqueue redundant work; existing vector writes/keyword updates remain responsible for repeat execution semantics. The real deployment verification used economy indexing, not every vector backend.

@github-actions github-actions Bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Nov 1, 2026
@github-actions github-actions Bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels Nov 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Retrying a segment status change after broker failure succeeds without restoring its index

1 participant