Skip to content

Carry the caller's context into render_scene and refresh worker pools (#80) - #85

Merged
thorwhalen merged 2 commits into
mainfrom
fix/80-pool-context
Oct 14, 2026
Merged

thorwhalen merged 2 commits into
mainfrom
fix/80-pool-context

Conversation

@thorwhalen

@thorwhalen thorwhalen commented Oct 14, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #80.

iter_render_scene (concurrency > 1) and refresh_full_docs handed work to a ThreadPoolExecutor without copying the caller's context, so a key bound with using_fal_credentials was invisible in the workers and render_scene's fal calls fell back to the operator's env key. Each task is now submitted as copy_context().run(fn, ...) (one copy per task; a Context cannot be entered by two threads at once), the same as execute_plan. refresh_full_docs makes only unauthenticated doc fetches today, so it is a consistency fix there.

Tests (fake transport, made-up marker key, no network): a render_scene worker authenticates with the bound key via SyncClient(key=...) rather than the module-level env path; the refresh pool sees the caller's context. Both fail on main, pass here. Full suite: 707 passed, 1 skipped; ruff clean.

Independent review (fresh Opus subagent, tried to break the diff): land it. No defect in the key-carrying fix; no other thread/pool hand-off in falaw loses context (execute_plan already copies); no key leaks between concurrent callers (copy taken at submit time); the new tests fail on main (checked in a main worktree). One confirmed regression in my first version: swapping pool.map for submit in refresh_full_docs stopped cancelling pending fetches after the first error (40 of 40 fetched vs 3 of 40). Fixed: pool.map over per-task context copies. Caveat, not a defect: iter_render_scene is a generator, so the context is captured at the first next(), not at the call.

thorwhalen and others added 2 commits October 14, 2026 12:52
…#80)

A ThreadPoolExecutor worker does not inherit the submitting thread's
contextvars, so a BYO fal key bound with using_fal_credentials was invisible
in render_scene's workers and the calls fell back to the operator's env key.
Submit each task through its own copy_context().run, as execute_plan does.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…text (#80 review)

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@thorwhalen
thorwhalen merged commit 14eff54 into main Oct 14, 2026
12 checks passed
@thorwhalen
thorwhalen deleted the fix/80-pool-context branch October 14, 2026 12:56
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.

render_scene / refresh thread pools drop the bound BYO fal key (context not copied)

1 participant