Update add-model skill: lagging-provider checks and push-gate rules#1281
Update add-model skill: lagging-provider checks and push-gate rules#1281tawnymanticore merged 4 commits intomainfrom
Conversation
📝 WalkthroughWalkthroughAdds Phase 1B guidance to run a provider-specific backfill cross-check on every skill invocation (pulling the 10 newest Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request updates the SKILL.md documentation to provide guidance on handling the Claude Code Web stop hook and defines a gating process before pushing changes. The feedback suggests enhancing the abandonment procedure by explicitly deleting local branches to ensure a clean state and including authentication errors as critical failures in the push gate criteria.
📊 Coverage ReportOverall Coverage: 91% Diff: origin/main...HEADNo lines with coverage information in this diff.
|
There was a problem hiding this comment.
🧹 Nitpick comments (3)
.agents/skills/claude-maintain-models/SKILL.md (3)
537-537: Minor wording: simplify the Together API key instruction.Line 537 says "ask the user before prompting them to export it" — the "before prompting them" is redundant. Consider:
-If the key isn't set, ask the user before prompting them to export it — don't fail silently onto models.dev. +If the key isn't set, ask the user to export it — don't fail silently onto models.dev.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/skills/claude-maintain-models/SKILL.md at line 537, Edit the SKILL.md text that currently reads "If the key isn't set, ask the user before prompting them to export it — don't fail silently onto models.dev." and simplify the phrasing by removing the redundant "before prompting them" (e.g., "If the key isn't set, ask the user to export it — don't fail silently onto models.dev."). Update the single line at or around the sentence in the Together API key instructions to the simplified wording.
60-75: Phase 1B design looks good; consider frequency optimization.The backfill check design is sound and addresses the "lagging providers" gap mentioned in the PR objectives. Running it on every invocation ensures recent models don't miss provider additions, which aligns with keeping the model list current.
One observation: checking 10 models across 3 providers means ~30 API calls every time the skill runs. If this becomes a bottleneck, consider throttling (e.g., only run Phase 1B if the user explicitly invokes discovery mode, or cache results for 24 hours). For now, the thoroughness seems justified.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/skills/claude-maintain-models/SKILL.md around lines 60 - 75, Phase 1B currently runs the lagging-provider backfill check on every invocation by pulling the top 10 entries from built_in_models in ml_model_list.py and cross-checking Fireworks/Together/SiliconFlow against KilnModel entries, which can cost ~30 API calls per run; modify the skill to reduce frequency by either gating Phase 1B behind an explicit "discovery mode" flag or adding a 24-hour cache for the backfill results keyed by provider+model (invalidate on model-add operations), and ensure the logic that reads built_in_models and compares against KilnModel honors this gate/cache so behavior remains identical when the check runs.
522-524: Consider adding language specifiers to code blocks.Static analysis flags these
WebFetchcommand blocks as missing language specifiers (MD040). WhileWebFetchisn't bash, you could either:
- Add
```bashif these are meant to be run in a shell context- Add a generic
```textor```shellfor consistency- Leave unmarked if these are intentionally tool-specific commands
Other WebFetch examples in the document (lines 522-524, 540-542) are inconsistent, so standardizing would improve clarity.
Also applies to: 540-542
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/skills/claude-maintain-models/SKILL.md around lines 522 - 524, Update the fenced code blocks that contain the WebFetch command (e.g., the block with "WebFetch https://fireworks.ai/models/fireworks/{model-slug}") to include a language specifier for consistency (choose one such as ```text, ```shell, or ```bash) and apply the same specifier to the other WebFetch examples in this document so all WebFetch blocks use the same tag; ensure you only modify the triple-backtick opening fence to add the chosen specifier for each WebFetch block (search for the literal "WebFetch" blocks to find all occurrences).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.agents/skills/claude-maintain-models/SKILL.md:
- Line 537: Edit the SKILL.md text that currently reads "If the key isn't set,
ask the user before prompting them to export it — don't fail silently onto
models.dev." and simplify the phrasing by removing the redundant "before
prompting them" (e.g., "If the key isn't set, ask the user to export it — don't
fail silently onto models.dev."). Update the single line at or around the
sentence in the Together API key instructions to the simplified wording.
- Around line 60-75: Phase 1B currently runs the lagging-provider backfill check
on every invocation by pulling the top 10 entries from built_in_models in
ml_model_list.py and cross-checking Fireworks/Together/SiliconFlow against
KilnModel entries, which can cost ~30 API calls per run; modify the skill to
reduce frequency by either gating Phase 1B behind an explicit "discovery mode"
flag or adding a 24-hour cache for the backfill results keyed by provider+model
(invalidate on model-add operations), and ensure the logic that reads
built_in_models and compares against KilnModel honors this gate/cache so
behavior remains identical when the check runs.
- Around line 522-524: Update the fenced code blocks that contain the WebFetch
command (e.g., the block with "WebFetch
https://fireworks.ai/models/fireworks/{model-slug}") to include a language
specifier for consistency (choose one such as ```text, ```shell, or ```bash) and
apply the same specifier to the other WebFetch examples in this document so all
WebFetch blocks use the same tag; ensure you only modify the triple-backtick
opening fence to add the chosen specifier for each WebFetch block (search for
the literal "WebFetch" blocks to find all occurrences).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 937d76cc-beeb-4011-89c5-43264b43b076
📒 Files selected for processing (1)
.agents/skills/claude-maintain-models/SKILL.md
* KIL-517 Fix misc spec builder bugs and improvements Addresses 11 items: add X button to dismiss questions, preserve answers on failed request, add Created At to spec details, allow whitespace while typing spec names (trim on submit), add priority selector in advanced options, fix autoselect badge persistence, rename FewShotSelector to TaskSampleSelector, fine tune page max-width, add Re-run button for review examples, disable copilot when full trace enabled, and add archive/unarchive to spec details. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Address Gemini review: use specific question numbers in validation messages Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Address CodeRabbit review: persist dismissed questions across remounts Lift dismissed state to parent like selections/other_texts so dismissals survive component remounts on API failures. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * KIL-522 Restore persisted model selection on Run page Initialize model from ui_state store (localStorage) instead of empty string so the previously selected model is restored on page load. Also fix the saved-config dropdown to show "custom" immediately instead of "Select an option" while configs load. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * KIL-522 Add one-shot guard to prevent default config from overriding intentional Custom selection Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * KIL-534 Add Feedback data model on TaskRun Replace the single `user_feedback` string field on TaskRun with a proper Feedback model that supports multiple feedback entries per run. Feedback is a parented model under TaskRun, stored as separate files to avoid write conflicts when multiple people provide feedback. - Add Feedback model (feedback text + FeedbackSource enum) - Make TaskRun a parent model with feedback children - Remove user_feedback field from TaskRun - Add REST API endpoints (list/create) for feedback on task runs - Update copilot models, utils, and frontend spec builder - Create follow-up ticket KIL-537 for repair UI replacement Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Add agent policy annotations for feedback API endpoints Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Revert unintended user_feedback renames in copilot code The ticket only asked to remove user_feedback from TaskRun, not rename it in the copilot/spec-builder code which uses it for a different purpose. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Remove misplaced annotation files, revert copilot renames Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Preserve feedback from spec review as Feedback children When creating TaskRuns from reviewed examples in the copilot flow, create Feedback children (with source=spec-feedback) after saving the run, so review feedback is not lost. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * reverts * KIL-537 Replace repair UI with feedback UI Remove all repair UI code (repair form, repair edit form, repair review/accept/delete flows) and replace with a new feedback UI that uses the Feedback data model from KIL-534. - Rename "Output Rating" to "Rating and Feedback" - Add inline feedback list (up to 3, truncated) with "Add Feedback" link - Add "All Feedback" modal with sortable table - Add "Add Feedback" modal using FormContainer - Delete output_repair_edit_form.svelte - Remove model_name/provider/focus_repair_on_appear props from Run Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Address AI review feedback: race condition and submit loading state - Add request ID tracking and run ID dedup to load_feedback to prevent race conditions and redundant requests when switching runs - Set add_feedback_submitting = true at start of submit_feedback Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Show latest 3 feedbacks in inline preview instead of oldest Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * reverted some changes * fixed add feedback dialog UI * outline instead of bg for clickable area * claude compatible mcp.json * steveback * policy anno * Add Fireworks AI provider to GLM 5.1 (#1275) https://getkiln.slack.com/archives/C0AG8U78MNG/p1776274097954549?thread_ts=1776273210.799549&cid=C0AG8U78MNG Co-authored-by: Claude <noreply@anthropic.com> * Add Grok 4.20 and Minimax M2.7 (Together AI) (#1269) * Add Grok 4.20 and Minimax M2.7 TogetherAI provider Added Grok 4.20 (OpenRouter) and TogetherAI provider for Minimax M2.7 to the model list. https://claude.ai/code/session_01S77zSCTFnNW52JiCyWpBoV * Remove reasoning flags from Grok 4.20 Other Grok models on OpenRouter don't set reasoning_capable=True. The model doesn't reliably return reasoning, causing 5 test failures. Removing to match the Kiln pattern for Grok on OpenRouter. https://claude.ai/code/session_01S77zSCTFnNW52JiCyWpBoV * Fix Minimax M2.7 Together AI structured output config The json_schema mode was being ignored by M2.7 on Together AI (model returned plain text instead of JSON). Switch to json_instruction_and_object with reasoning_optional_for_structured_output and optional_r1_thinking parser, matching the M2.5 Together AI config that works reliably. https://claude.ai/code/session_01F1L5ryuY5t2MxQXbNVjQGj --------- Co-authored-by: Claude <noreply@anthropic.com> * Update add-model skill: lagging-provider checks and push-gate rules (#1281) * Update SKILL.md * Update SKILL.md * Update SKILL.md * CR * Workaround for Claude Code web for using anthropic models in paid tests (#1283) * Update SKILL.md * Update SKILL.md * Update SKILL.md * CR * Update SKILL.md * Add Claude Opus 4.7 to model list (#1282) * Add Claude Opus 4.7 to model list (anthropic, openrouter) Adds Anthropic's new Opus 4.7 model with both Anthropic and OpenRouter providers. Introduces CLAUDE_OPUS_4_7_ANTHROPIC_THINKING_LEVELS to support the new "xhigh" and "max" effort levels exclusive to Opus 4.7. * Apply zero-sum swap: demote Opus 4.6 from suggested/featured Opus 4.7 now carries featured_rank=2, editorial_notes, suggested_for_evals, and suggested_for_data_gen. Removing the same flags from Opus 4.6 keeps the suggested/featured count stable across the Claude Opus family. https://claude.ai/code/session_01Xnfzt91McoMdqaiRv1g6xg * Add PDF support to OpenRouter provider for Opus 4.7 Adds KilnMimeType.PDF to multimodal_mime_types and sets multimodal_requires_pdf_as_image=True (OpenRouter's PDF routing through Mistral OCR breaks LiteLLM parsing, so PDFs must be sent as images). https://claude.ai/code/session_01Xnfzt91McoMdqaiRv1g6xg --------- Co-authored-by: Claude <noreply@anthropic.com> --------- Co-authored-by: Sam Fierro <13154106+sfierro@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: scosman <scosman@users.noreply.github.com>
What does this PR do?
The stop hook in Claude Code web is super annoying. "hopeful" fix.
Checklists
Summary by CodeRabbit