fix: real async embeddings for goal alignment; unblock event loop in planner - #42
Merged
Conversation
…e event loop The goal-alignment feature was silently meaningless in production: generate_embedding_sync cannot drive the async OpenAI client from inside a running event loop (always the case under FastAPI), so it caught the RuntimeError and fell back to a deterministic SHA-1 hash pseudo-embedding. Cosine similarity over hash digests is garbage, so every 'related initiative' surfaced — and every Slack notification fired — was based on noise, with no externally visible failure. Make the embedding path async end-to-end instead: the planner awaits the async embedding API directly, the hash stub and the sync wrapper are deleted, and when embeddings cannot be generated (API failure or dry-run mode with no client) goal alignment is skipped with a logged warning rather than faked. Also replace Runner.run_sync with await Runner.run in the planner/critic agent calls and thread async signatures up through generate_plan, generate_plan_for_idea, the /plan endpoints, and the procedure runner, so LLM calls no longer block the event loop. The goal-alignment Slack notification is now awaited directly instead of the sync/async asyncio.run branching.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
generate_embedding_synccould never call the async OpenAI embeddings API from inside a running event loop (i.e. always, under FastAPI), so it fell back to a deterministic SHA-1 hash pseudo-embedding. Goal alignment was computing cosine similarity over hash digests — every "related initiative" and Slack notification was based on garbage similarity, with no externally visible failure. The stub path and sync wrapper are deleted; embeddings are now generated via the async API end-to-end.generate_embeddingreturnsNonein dry-run mode and raises on API failure; the planner treats both as "skip alignment".Runner.run_sync(...)→await Runner.run(...)inagent_sdk.py(verified against installedopenai-agents0.3.3:Runner.runis a coroutine function with the same kwargs).generate_plan/generate_plan_for_idea/_collect_related_goals/_notify_alignmentare now async, awaited from the/planendpoints and the procedure runner (which no longer needsasyncio.to_thread). The alignment Slack notification is awaited directly instead of theasyncio.run/create_taskbranching.Test plan
uv run ruff check .— passuv run ruff format --check .— passuv run mypy agent_pm— passuv run pytest -q— 135 passed (was 134)_RUNNER.run,generate_embedding, async planner/critic fakes); dry-run embedding test now assertsNoneinstead of a 1536-dim stub.test_goal_alignment_skipped_when_embeddings_unavailable: embedding failure →related_initiatives == [], no "Related Initiatives" section in the PRD, no notification dispatched, nogoal_alignmenttrace event — proving no hash-fallback path remains.