Skip to content

fix(retain): preserve initial call with zero retry budget#2741

Closed
koriyoshi2041 wants to merge 1 commit into
vectorize-io:mainfrom
koriyoshi2041:rios/retain-zero-retry-initial-call
Closed

fix(retain): preserve initial call with zero retry budget#2741
koriyoshi2041 wants to merge 1 commit into
vectorize-io:mainfrom
koriyoshi2041:rios/retain-zero-retry-initial-call

Conversation

@koriyoshi2041

Copy link
Copy Markdown
Contributor

Summary

  • make fact extraction perform one initial request when retain_llm_max_retries=0
  • keep forwarding the configured zero transport retry budget to the provider
  • report the actual outer attempt count in the exhausted-attempt fallback

Closes #2731.

Validation

  • uv run --package hindsight-api-slim pytest hindsight-api-slim/tests/test_fact_extraction_retry.py -q (13 passed)
  • uv run --package hindsight-api-slim ruff check hindsight-api-slim/hindsight_api/engine/retain/fact_extraction.py hindsight-api-slim/tests/test_fact_extraction_retry.py
  • uv run --package hindsight-api-slim ruff format --check hindsight-api-slim/hindsight_api/engine/retain/fact_extraction.py hindsight-api-slim/tests/test_fact_extraction_retry.py
  • git diff --check

@ebarkhordar ebarkhordar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The retry change itself is two files and lines up with what #2731 asks for. One note on the branch, not on the code.

This branch is stacked on your #2669 but targets main. The head's only parent is #2669's head, which is not on main yet:

$ gh api repos/vectorize-io/hindsight/commits/435874aa3 --jq ".parents[].sha"
ab9a9546db395b780a4997688336578000004fb7      # head of #2669, unmerged

$ gh api repos/vectorize-io/hindsight/compare/main...ab9a9546d --jq ".status"
diverged

That is where the "strict_schema": True hunk in consolidation/consolidator.py and its test come from. They are #2669's change, and a zero retry budget has nothing to do with them. So merging this into main as it stands would also land #2669, reviewed as a side effect of a retry fix rather than on its own.

Retargeting this PR's base to rios/consolidation-strict-schema would show only the retry commit here and keep the two landing in order. Rebasing onto main after #2669 merges does the same. Either way #2669 keeps its own review.

Flagging it because the two PRs are easy to merge out of order, not because anything in the retry fix looks wrong to me.

For transparency: I used an AI agent to check this. The two SHAs above are from the API calls shown, and I have not run the test suite here.

@koriyoshi2041
koriyoshi2041 force-pushed the rios/retain-zero-retry-initial-call branch from 435874a to c1cf235 Compare July 16, 2026 14:37
@koriyoshi2041

Copy link
Copy Markdown
Contributor Author

Good catch. I rebased this branch directly onto current main, so #2741 now contains only the two-file zero-retry fix; the #2669 strict-schema change is no longer in its history or diff.

New head: c1cf2357

Revalidated:

  • uv run --package hindsight-api-slim pytest hindsight-api-slim/tests/test_fact_extraction_retry.py -q — 13 passed
  • focused Ruff check and format check
  • git diff --check origin/main...HEAD

@nicoloboschi nicoloboschi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uhm this fix is wrong. with llm_max_retries=1, we should do 1 retry still. so I think we should just do llm_max_retries+1 attempts - please also check the impl in llm_config.call follow the same pattern

@nicoloboschi

Copy link
Copy Markdown
Collaborator

closing for #2779 which is slightly better

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.

fix(retain): a zero retry budget must still perform the initial fact-extraction request

3 participants