Skip to content

feat: retry OpenRouter and fall back to deepseek-v3.2 - #14

Merged
tonyandrewmeyer merged 4 commits into
canonical:mainfrom
tonyandrewmeyer:openrouter-retry-fallback
Oct 8, 2026
Merged

tonyandrewmeyer merged 4 commits into
canonical:mainfrom
tonyandrewmeyer:openrouter-retry-fallback

Conversation

@tonyandrewmeyer

@tonyandrewmeyer tonyandrewmeyer commented Oct 8, 2026 •

Copy link
Copy Markdown
Collaborator

The enricher now retries a rate limit, 5xx, timeout or dropped connection up to twice, and asks OpenRouter for deepseek/deepseek-v3.2 when the configured model errors. The first scheduled failure in operator since the enricher was wired up (run 37491405242, commented on canonical/operator#2750) got a single 429 back from OpenRouter, so it posted the plain body. deepseek/deepseek-chat currently has only two providers that support structured output, so one of them rate-limiting halves the pool (v3.2 has ten).

  • Waits are 5s then 15s, or the Retry-After if OpenRouter sends one, capped at 30s each and a minute in total. Each attempt keeps the existing 60s timeout, so the worst case is about four minutes of runner time. Other 4xx aren't retried, because a 400 is the request and a 402 is the key's credit.
  • The fallback uses OpenRouter's models list (deepseek-chat then v3.2, with OPENROUTER_MODEL ahead of both if it's set, rather than in place of deepseek-chat), which only moves on when a model errors. The request now sends models without model, which is what the fallback docs show.
  • The model that answered goes in the job log and in a separate hidden <!-- ai-failure-notifications:model ... --> comment next to the marker. None of the existing marker parsers match it, so artefacts written by the current release still parse and older releases can read the new ones.

I ran v3.2 over the canary fixtures (all 18 cells, three times each, with v3.2 as the primary so every answer was its own). It drops and attaches the same duplicates as deepseek-chat, it never commented on the wrong issue or invented one, and every envelope was valid. However, every reply from two of its providers (Venice and DeepInfra) came back wrapped in a ```json fence that json.loads can't parse, so the last commit unwraps a fence around the whole reply before validating it. The package's own retry never fired, because OpenRouter moved the 429s to v3.2 itself, so that path is only covered by the unit tests.

Retry a 429, a 5xx, a timeout or a dropped connection up to twice, waiting
5s then 15s (or the Retry-After, capped at 30s each and 60s in total).
Other 4xx are not retried. The final error says how many attempts were made.

Send OpenRouter a `models` list, the configured model first and
FALLBACK_MODELS after it, so a rate-limited provider for the first model
moves the request to the next. The model that answered is logged and written
into enriched artefacts as a separate hidden comment beside the marker, which
none of the existing marker parsers match.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SSoK94zL2ZUrBqUEQrFnyp
@tonyandrewmeyer
tonyandrewmeyer marked this pull request as ready for review October 8, 2026 01:22
Nothing in production reads the model stamp back, so parse_model_stamp() and
MODEL_STAMP_RE only served the tests, which now assert on the rendered stamp.
The fallback comment no longer carries provider counts that will drift, and a
test docstring now names 402, OpenRouter's insufficient-credits status, rather
than 403.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@james-garner-canonical james-garner-canonical 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.

Makes sense. One non-blocking concern about how OPENROUTER_MODEL replaces one default but not the rest -- I think it should either prepend (not replace), or be a list that replaces all the default models. The latter is probably safer if the use case is to control what models are requested, but prepending makes sense if the use case is only to control what model is tried first.

Comment thread ai-failure-notifier/src/charm_tech_code/ai_failure_notifier/_constants.py Outdated
tonyandrewmeyer and others added 2 commits October 8, 2026 15:21
Some providers return the JSON envelope wrapped in a ```json fence even under
the strict json_schema response format. On the canary fixtures, every
deepseek-v3.2 reply from two of its providers came back that way, and
json.loads failed on each, so each would have been posted as the plain body.
Only a fence around the whole reply is removed, and the envelope is still
validated afterwards.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…lacing one

The default and fallback models are now one DEFAULT_MODELS list, chat then
v3.2, and OPENROUTER_MODEL, if set, is tried before them rather than replacing
only the first. With it unset (or empty, as an unset Actions variable is) the
request is unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@tonyandrewmeyer
tonyandrewmeyer merged commit a9da37c into canonical:main Oct 8, 2026
11 checks passed
@tonyandrewmeyer
tonyandrewmeyer deleted the openrouter-retry-fallback branch October 8, 2026 04:39
tonyandrewmeyer added a commit to canonical/operator that referenced this pull request Oct 8, 2026
…ons (#2808)

Updates the `canonical/charm-tech-code` pins in the release and AI
failure notification workflows to
[`cc25c09`](canonical/charm-tech-code@cc25c09),
the current `main`, and uses the new `draft-release-notes` options.

`ai-failure-notifier` (was `bcbd3cc`):
* Retries OpenRouter on rate limits, 5xx errors and timeouts, and falls
back to `deepseek/deepseek-v3.2` (canonical/charm-tech-code#14).
* Records the model that answered in a hidden comment on the enriched
issue or comment.

`changelog` and `draft-release-notes` (were `61b97db`):
* Finds release markers in a description with CRLF line endings
(canonical/charm-tech-code#15).
* Applies the review feedback from the 3.9.0 release
(canonical/charm-tech-code#16): leaves out security advisory merges,
puts code in summaries in backticks, ends the release body with a thanks
line for contributors, renames the compare line to "All commits", and
updates the release notes prompt.

Changes in this repository:
* `propose-release.yaml` writes the range's commit messages to a file
and passes it to `draft-release-notes` with `--commits`, along with
`--docs-url https://canonical.com/juju/docs/ops/latest`. The model then
sees the PR descriptions rather than only the changelog, and gets the
published URL of any docs page a commit changed, so it can link new
pages.
* `CONTRIBUTING.md` and a warning in `create-draft-release.yaml`
describe the new end of the release body ("All commits" and the thanks
line) rather than "Full Changelog".

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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.

3 participants