Skip to content

fix: avoid retrying invalid embedding dimensions#1687

Open
ai-ag2026 wants to merge 2 commits into
vectorize-io:mainfrom
ai-ag2026:fix/nonretryable-invalid-embedding-dimensions
Open

fix: avoid retrying invalid embedding dimensions#1687
ai-ag2026 wants to merge 2 commits into
vectorize-io:mainfrom
ai-ag2026:fix/nonretryable-invalid-embedding-dimensions

Conversation

@ai-ag2026
Copy link
Copy Markdown

@ai-ag2026 ai-ag2026 commented May 21, 2026

Summary

Classify deterministic embedding-dimension failures as non-retryable worker errors, so poisoned embedding responses fail immediately instead of burning worker slots through the generic retry path.

Context

This complements #1670, but no longer depends on it: this PR now applies cleanly on current main and catches both the preflight validation error shape and pgvector's native dimension-mismatch error shape.

Changes

  • Extract _is_non_retryable_task_error() for deterministic worker failure classification.
  • Preserve existing asyncpg/Oracle integrity-violation non-retry behavior.
  • Add invalid embedding-dimension classification for:
    • embedding 0 has dimension 0; expected 384
    • different vector dimensions 384 and 0
  • Keep transient non-integrity errors on the retry path.

Verification

  • Rebased onto current origin/main (113d7da9) and cherry-picked only fix: avoid retrying invalid embedding dimensions.
  • env HINDSIGHT_API_LLM_PROVIDER=mock HINDSIGHT_API_LLM_MODEL=mock HINDSIGHT_API_LLM_API_KEY=mock uv run --extra embedded-db --extra local-ml pytest -n0 tests/test_integrity_violation_not_retried.py -q → 5 passed, 2 warnings
  • uv run ruff check hindsight_api/engine/memory_engine.py tests/test_integrity_violation_not_retried.py → passed
  • uv run python -m py_compile hindsight_api/engine/memory_engine.py tests/test_integrity_violation_not_retried.py → passed
  • Added-line hygiene scan → 0 secret/shell/eval/pickle hits

AI usage disclosure

Prepared with AI assistance (TARS/Codex) using TDD, targeted local tests, and manual diff review.

@ai-ag2026 ai-ag2026 force-pushed the fix/nonretryable-invalid-embedding-dimensions branch from 5e332b6 to 8259d6a Compare May 21, 2026 15:40
@ai-ag2026 ai-ag2026 marked this pull request as ready for review May 21, 2026 15:40
@benfrank241
Copy link
Copy Markdown
Contributor

@ai-ag2026 quick coordination question on this and #1670:

since #1687's classifier covers both the pgvector native error (firing on main today) and the preflight validator from #1670 (not yet merged), is there a preferred merge order in your head? specifically:

  1. land fix: validate embedding dimensions before pgvector writes #1670 first so this PR's "; expected" match arm has real coverage in tests, then fix: avoid retrying invalid embedding dimensions #1687
  2. land fix: avoid retrying invalid embedding dimensions #1687 first (covers pgvector native case standalone), fix: validate embedding dimensions before pgvector writes #1670 follows when ready
  3. either order works

happy with whichever — just want to make sure they don't end up in a half-merged state where one waits indefinitely on the other.

@ai-ag2026
Copy link
Copy Markdown
Author

Updated this PR with the generated-file refresh from the CI failure:

  • reran ./scripts/generate-openapi.sh
  • reran ./scripts/generate-bank-template-schema.sh
  • reran ./scripts/generate-clients.sh
  • reran ./scripts/generate-docs-skill.sh
  • reran ./scripts/hooks/lint.sh

That produced the three expected generated-file deltas and the branch now points at 37e3accd.

On merge order with #1670: I think this PR can land first. The classifier here covers the pgvector-native dimension mismatch that can fire on current main today, and it also covers the preflight validator path from #1670 if/when that lands later. So #1670 complements this, but this PR does not need to wait on it.

Local verification after regeneration:

  • ./scripts/hooks/lint.sh passed during generation / pre-commit
  • HINDSIGHT_API_LLM_PROVIDER=none HINDSIGHT_API_SKIP_LLM_VERIFICATION=true uv run pytest tests/test_integrity_violation_not_retried.py -q → 5 passed

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.

2 participants