Model Availability Polling and ELO-Based Dynamic Routing - #123
Conversation
…outing (#119) - Replaced rigid bash picking logic inside GHA model router with `scripts/model_router.py`. - Implemented real-time OpenRouter `:free` model availability polling via live API endpoint. - Introduced custom performance index (ELO ≈ 3L0) in `docs/schemas/model-success-matrix.yaml` to phase out reliance on public leaderboards. - Maintained 100% GHA cache budget compatibility and added comprehensive pytest unit tests under `tests/test_model_router.py`.
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Mention Blocks like a regular teammate with your question or request: @blocks review this pull request Run |
|
head_sha: d82fb4f Peer review gate (ready for second-pass agents)External reviewers polled: CodeRabbit, Devin, Aikido, Sentry, Copilot. Peer activity (truncated): Downstream: |
|
@jules Auto-resolve (GHA agent-review-auto-jules) — do not wait for a human ping. Feedback excerptInstructions
|
This comment was marked as off-topic.
This comment was marked as off-topic.
…119) - Replaced rigid bash picking logic inside GHA model router with `scripts/model_router.py`. - Implemented real-time OpenRouter `:free` model availability polling via live API endpoint. - Introduced custom performance index (ELO ≈ 3L0) in `docs/schemas/model-success-matrix.yaml` to phase out reliance on public leaderboards. - Catch Linear 'usage limit exceeded' / 'USAGE_LIMIT_EXCEEDED' GraphQL errors gracefully in `agent-feedback-linear-sync.yml` to prevent CI failure when free tiers are exhausted. - Maintained 100% GHA cache budget compatibility and added comprehensive pytest unit tests under `tests/test_model_router.py`.
|
head_sha: 3bedc85 Peer review gate (ready for second-pass agents)External reviewers polled: CodeRabbit, Devin, Aikido, Sentry, Copilot. Peer activity (truncated): Downstream: |
This comment was marked as off-topic.
This comment was marked as off-topic.
Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…ching, robust fallback, security checks, and graceful Linear sync limit handling (#119) - Replaced rigid GHA bash router with `scripts/model_router.py`. - Added 1-hour temporal file caching for OpenRouter `:free` model list polling to optimize polling timing/intervals. - Guarded polling behind `has_openrouter` to completely eliminate redundant network latency on restricted runners. - Asserted `:free` suffix check on all selected OpenRouter models for robust safety enforcement. - Wrapped `main()` in a top-level try-except for seamless graceful degradation inside actions. - Moved comment reaction inside try-except for Linear issue synchronization in `agent-feedback-linear-sync.yml` to prevent fake success reactions on limit errors. - Added comprehensive unit tests in `tests/test_model_router.py` (all passed).
|
@jules @google-labs-jules @coderabbitai Evaluate and fix everything necessary for performant runs executed in this PR; including: Make sure everything is either maintained or upgraded. |
- Replaced rigid GHA bash router with `scripts/model_router.py`. - Added 1-hour temporal file caching for OpenRouter `:free` model list polling to optimize polling timing/intervals. - Guarded polling behind `has_openrouter` to completely eliminate redundant network latency on restricted runners. - Asserted `:free` suffix check on all selected OpenRouter models for robust safety enforcement. - Wrapped `main()` in a top-level try-except for seamless graceful degradation inside actions. - Moved comment reaction inside try-except for Linear issue synchronization in `agent-feedback-linear-sync.yml` to prevent fake success reactions on limit errors. - Added comprehensive unit tests in `tests/test_model_router.py` (all passed). - Updated `docs/proposals/active/rate-limit-rotation/ITEMS.md` tracking row. Implements: RL-17 Signed-off-by: Jules <jules@grok.x.ai>
|
head_sha: 2be3c17 Peer review gate (ready for second-pass agents)External reviewers polled: CodeRabbit, Devin, Aikido, Sentry, Copilot. Peer activity (truncated): Downstream: |
|
@jules Auto-resolve (GHA agent-review-auto-jules) — do not wait for a human ping. Feedback excerptInstructions
|
🔀 OpenRouter review (
|
- Replaced rigid GHA bash router with `scripts/model_router.py`. - Added 1-hour temporal file caching for OpenRouter `:free` model list polling to optimize polling timing/intervals. - Guarded polling behind `has_openrouter` to completely eliminate redundant network latency on restricted runners. - Asserted `:free` suffix check on all selected OpenRouter models for robust safety enforcement. - Wrapped `main()` in a top-level try-except for seamless graceful degradation inside actions. - Moved comment reaction inside try-except for Linear issue synchronization in `agent-feedback-linear-sync.yml` to prevent fake success reactions on limit errors. - Added comprehensive unit tests in `tests/test_model_router.py` (all passed). - Updated `docs/proposals/active/rate-limit-rotation/ITEMS.md` tracking row. Implements: RL-17 Signed-off-by: Jules <jules@grok.x.ai>
This comment was marked as off-topic.
This comment was marked as off-topic.
- Replaced rigid GHA bash router with `scripts/model_router.py`. - Added 1-hour temporal file caching for OpenRouter `:free` model list polling to optimize polling timing/intervals. - Guarded polling behind `has_openrouter` to completely eliminate redundant network latency on restricted runners. - Asserted `:free` suffix check on all selected OpenRouter models for robust safety enforcement. - Wrapped `main()` in a top-level try-except for seamless graceful degradation inside actions. - Moved comment reaction inside try-except for Linear issue synchronization in `agent-feedback-linear-sync.yml` to prevent fake success reactions on limit errors. - Added comprehensive unit tests in `tests/test_model_router.py` (all passed). - Updated `docs/proposals/active/rate-limit-rotation/ITEMS.md` tracking row. Implements: RL-17 Signed-off-by: Jules <jules@grok.x.ai>
timerloggedout-spec
left a comment
There was a problem hiding this comment.
Approved as OPERATOR. Model availability polling + ELO dynamic routing advances RL-05 / #122. Devin findings noted for follow-up; unit tests reported green. Merge when checks clear.
|
OPERATOR merge complete. #123 (Model Availability Polling + ELO Dynamic Routing) merged to
All open review threads resolved prior to merge. Follow-up: rebase #104 / #105 / #114 onto new master; sync model-rotation.yaml with scripts/model_router.py limits (noted in Devin analysis). |
| if "GITHUB_OUTPUT" in os.environ: | ||
| try: | ||
| with open(os.environ["GITHUB_OUTPUT"], "a") as go: | ||
| go.write("provider=none\n") | ||
| go.write("model=\n") | ||
| go.write("skip=true\n") | ||
| go.write(f"reason=Model Router crashed: {e}\n") | ||
| except Exception: | ||
| pass |
There was a problem hiding this comment.
🟡 Model selection step can hard-fail when the router hits an unexpected error
The router's emergency "no capacity" result is appended as raw text (go.write(f"reason=Model Router crashed: {e}\n") at scripts/model_router.py:445) without the multi-line-safe delimiter format, so an error message that spans more than one line makes the whole step fail instead of degrading gracefully.
Impact: Instead of quietly skipping AI triage/review, the workflow job errors out, so pull requests and issues get a red check.
Mechanism: GITHUB_OUTPUT key=value lines must be single-line or use a heredoc delimiter
The whole point of the top-level except in scripts/model_router.py:429-447 is graceful degradation. But GitHub Actions parses $GITHUB_OUTPUT line-by-line as key=value; any embedded newline in the value produces Error: Unable to process file command 'output' successfully. Invalid format, which fails the step (the composite step runs with bash -e and the runner reports the file-command error). Exception messages with newlines are entirely possible (e.g. json.JSONDecodeError-style or OSError text carrying embedded newlines from external data).
The safe pattern is the heredoc form already used elsewhere in the repo (.github/workflows/gemini-triage.yml:95-108 uses echo "body<<EOF" ... EOF), or simply sanitising newlines out of the reason string before writing.
| if "GITHUB_OUTPUT" in os.environ: | |
| try: | |
| with open(os.environ["GITHUB_OUTPUT"], "a") as go: | |
| go.write("provider=none\n") | |
| go.write("model=\n") | |
| go.write("skip=true\n") | |
| go.write(f"reason=Model Router crashed: {e}\n") | |
| except Exception: | |
| pass | |
| if "GITHUB_OUTPUT" in os.environ: | |
| try: | |
| safe_reason = f"Model Router crashed: {e}".replace("\r", " ").replace("\n", " ") | |
| with open(os.environ["GITHUB_OUTPUT"], "a") as go: | |
| go.write("provider=none\n") | |
| go.write("model=\n") | |
| go.write("skip=true\n") | |
| go.write(f"reason={safe_reason}\n") | |
| except Exception: | |
| pass |
Was this helpful? React with 👍 or 👎 to provide feedback.
| # Cross-reference OpenRouter polled availability | ||
| if provider == "openrouter": | ||
| if polled_free_models is not None: | ||
| if model not in polled_free_models: | ||
| sys.stderr.write(f"Warning: OpenRouter model {model} is not currently available/free in polled catalog. Skipping.\n") | ||
| continue | ||
| else: | ||
| # No catalog (fresh or cached) available at all! Skip unverified new models conservatively. | ||
| if model not in LEGACY_MODELS: | ||
| sys.stderr.write(f"Warning: No OpenRouter catalog available. Skipping unverified new model '{model}' conservatively.\n") | ||
| continue |
There was a problem hiding this comment.
📝 Info: Conservative skip when the OpenRouter catalog is unreachable can starve the peer path
When neither a fresh nor a stale catalog is available, only ids in LEGACY_MODELS are allowed (scripts/model_router.py:326-330). For the review role the top-scored candidates cohere/north-mini-code:free and google/gemma-4-31b-it:free are not in that set, so a transient openrouter.ai outage silently pushes routing onto the legacy models and then onto the 15/day Gemini residuals. The behaviour is intentional and safe, but the stderr warnings are not surfaced as GitHub annotations (::warning::), so operators won't see why routing changed.
Was this helpful? React with 👍 or 👎 to provide feedback.
| original_parse_yaml = mr.parse_yaml | ||
| monkeypatch.setattr(mr, "parse_yaml", lambda path: original_parse_yaml(str(matrix_file)) if "success" in path else {}) |
There was a problem hiding this comment.
📝 Info: Test monkeypatches parse_yaml with a substring heuristic
monkeypatch.setattr(mr, "parse_yaml", lambda path: original_parse_yaml(str(matrix_file)) if "success" in path else {}) couples the test to the literal substring 'success' appearing in the production path constant (scripts/model_router.py:248). Renaming the schema file would make the test silently exercise an empty matrix (all scores default to 1000) instead of failing loudly. Making the schema path an injectable parameter/env var would remove the need for this patching entirely.
Was this helpful? React with 👍 or 👎 to provide feedback.
| } catch (err) { | ||
| const msg = err.message || ''; | ||
| if (msg.includes('usage limit exceeded') || msg.includes('USAGE_LIMIT_EXCEEDED')) { | ||
| core.warning('Linear workspace limit exceeded — skipping sync cleanly: ' + msg); | ||
| } else { | ||
| throw err; | ||
| } | ||
| } |
There was a problem hiding this comment.
📝 Info: Linear sync now swallows only string-matched usage-limit errors
The new top-level catch keys off msg.includes('usage limit exceeded') || msg.includes('USAGE_LIMIT_EXCEEDED') against err.message, but linear() throws new Error(JSON.stringify(json.errors)) (see the helper defined above at .github/workflows/agent-feedback-linear-sync.yml:85-87), so the match depends on Linear's exact GraphQL error payload wording remaining stable. If Linear changes the message (or returns the limit as an extensions.code only), the workflow will go back to failing the job. Matching on a broader signal (e.g. case-insensitive 'usage limit' / 'limit exceeded') would be more robust.
Was this helpful? React with 👍 or 👎 to provide feedback.
|
head_sha: 15391b0 Peer review gate (ready for second-pass agents)External reviewers polled: CodeRabbit, Devin, Aikido, Sentry, Copilot. Peer activity (truncated): Downstream: |
Implemented model availability polling for OpenRouter free models and transitioned to our internal historic performance index (ELO ≈ 3L0) to route triage, review, and invoke tasks dynamically. Verified using a comprehensive set of unit tests (all passed successfully).
Fixes #122
PR created automatically by Jules for task 3359099756114716988 started by @timerloggedout-spec