Skip to content

fix: real async embeddings for goal alignment; unblock event loop in planner - #42

Merged
haasonsaas merged 1 commit into
mainfrom
fix/real-embeddings-async
Jul 25, 2026
Merged

fix: real async embeddings for goal alignment; unblock event loop in planner#42
haasonsaas merged 1 commit into
mainfrom
fix/real-embeddings-async

Conversation

@haasonsaas

Copy link
Copy Markdown
Contributor

Summary

  • Kill the silent fake-embedding fallback. generate_embedding_sync could 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.
  • Failure behavior: when embeddings can't be generated (API error, or dry-run mode with no client), goal alignment is skipped with a logged warning — never faked. generate_embedding returns None in dry-run mode and raises on API failure; the planner treats both as "skip alignment".
  • Unblock the event loop. Runner.run_sync(...)await Runner.run(...) in agent_sdk.py (verified against installed openai-agents 0.3.3: Runner.run is a coroutine function with the same kwargs). generate_plan / generate_plan_for_idea / _collect_related_goals / _notify_alignment are now async, awaited from the /plan endpoints and the procedure runner (which no longer needs asyncio.to_thread). The alignment Slack notification is awaited directly instead of the asyncio.run/create_task branching.

Test plan

  • uv run ruff check . — pass
  • uv run ruff format --check . — pass
  • uv run mypy agent_pm — pass
  • uv run pytest -q — 135 passed (was 134)
  • Tests updated to mock the async APIs (_RUNNER.run, generate_embedding, async planner/critic fakes); dry-run embedding test now asserts None instead of a 1536-dim stub.
  • New test test_goal_alignment_skipped_when_embeddings_unavailable: embedding failure → related_initiatives == [], no "Related Initiatives" section in the PRD, no notification dispatched, no goal_alignment trace event — proving no hash-fallback path remains.

…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.
@haasonsaas
haasonsaas merged commit 04d5fae into main Jul 25, 2026
6 checks passed
@haasonsaas
haasonsaas deleted the fix/real-embeddings-async branch July 25, 2026 02:23
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