Skip to content

fix: retain parser admission until actual worker completion - #959

Draft
seonghobae wants to merge 1 commit into
feat/bounded-parse-admissionfrom
fix/parse-admission-work-lifetime-20260928
Draft

seonghobae wants to merge 1 commit into
feat/bounded-parse-admissionfrom
fix/parse-admission-work-lifetime-20260928

Conversation

@seonghobae

Copy link
Copy Markdown
Collaborator

Summary

Stacked follow-up to #548 at dd5ebcfac3f0163fb083741ae834d4f304cb4797.
Retain its existing authenticated, non-waiting parser lease until actual
synchronous worker completion after request cancellation. No new queue,
capacity setting, dependency, workflow or authentication policy.

Root cause and repair

Starlette runs the endpoint in a task separate from outer middleware.
Cancelling the outer ASGI request released the lease while its parser thread
continued. The actual ASGI regression observed a second 200 instead of 429.

Store the guarded default-executor completion future in shared request state
before yielding. Defer the existing lease release to exactly one completion
callback. Preserve copied context, temporary uploads, repeated cancellation and
orderly task-shutdown behavior. A completed future can conservatively retain
capacity for one additional loop turn.

Verification

  • Actual ASGI cancellation + live synchronous worker, both success and failure:
    second request remains 429/Retry-After 1/no-store, no extra worker starts,
    then capacity recovers with 200 after terminal work.
  • Repeated direct task cancellation, actual AnyIO cancel scopes and cancellation
    of all remaining event-loop tasks. Old worker-task implementation failed
    shutdown cleanup ordering; guarded executor completion passes.
  • Full suite: 536 passed, warnings as errors; required production API branch
    coverage 100% (893 statements, 264 branches).
  • New test module Ruff and git diff checks pass. Independent source review
    found no blocking issue; it did not rerun tests.

Acceptance limits

Draft while parent #548 remains CHANGES_REQUESTED and hosted checks/review are
unaccepted. This is a local authenticated ASGI fixture, not deployment or real
OCR acceptance. Capacity remains per application/process, not global across
replicas or shared model servers. Forced process termination is outside the
cleanup guarantee. Separate native OCR migration continues independently.

Optional wider API-plus-tools coverage was 96.15%, solely in unchanged
tools/benchmark_upload_ingestion.py; it is not claimed as passing. Both hosted
repository test gates explicitly require API-only coverage; no gate was changed.

Durable RCA: docs/doctoring/parser-cancellation-work-lifetime-20260928.md.

@coderabbitai

coderabbitai Bot commented Sep 27, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

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.

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant