Conversation
Once `aicraft` joins the tuple in `hosted_openai_compat_hosts_are_templates_not_enum_variants` the line exceeds the width, so `cargo fmt --all -- --check` rejected it and the required Lint check stayed red. Applied by a maintainer rather than sent back as a review round-trip. The contribution is unchanged; only its formatting. Pushing to the fork was refused despite maintainerCanModify, so this lands on an integration branch per AGENTS.md.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Claude finished @Hmbown's task in 5m 28s —— View job Reviewing PR #6288
SummaryThe AICraft descriptor row follows the existing SenseNova/Baseten/Groq/Cerebras Finding: new shipped default silently skips the pricing-coverage safety netThere is a second, parallel registry of the same "named OpenAI-compatible That JSON is exactly what Because AICraft isn't in Suggested fix: add an Other checks (no issues found)
|
|
Claude finished @Hmbown's task in 1m 43s —— View job Reviewing PR #6288
SummaryRe-checked the branch as of Finding: pricing-coverage safety net still doesn't see AICraft (unaddressed from prior review)This still stands and nothing in the current diff touches it:
Net effect: a user who picks the new AICraft template gets a live, selectable default ( Suggested fix (unchanged from prior review): add an Other checks (no issues found)
|
![codewhale-agent[bot]](https://avatars.githubusercontent.com/u/101357273?s=60&v=4){kind=small}
There was a problem hiding this comment.
Codewhale review
Complete review coverage: 1/1 passes, 2 file patches, sha256:8951e750a699a4cbcd3dffc58fbfa16925c558f011fd0f1e0fa605ceb51bd3a1.
Pass 1: Mechanical addition of an AICraft OpenAI-compatible template row (four consts + one ProviderSetupApply::Compatible entry) plus the matching test-string update and a docs table row. By inspection the diff is internally consistent: constants match the descriptor row, the new row sits before agnes, the settings_value() expectation includes aicraft in the same order the templates are declared, the new tuple in hosted_openai_compat_hosts_are_templates_not_enum_variants matches the declared consts, and the docs/PROVIDERS.md row mirrors the descriptor (AICraft, https://aicraftapi.com/v1, claude-4.6-sonnet, AICRAFT_API_KEY). No defect was demonstrated from the supplied evidence.
Assessment
Pass 1: No defect is demonstrable from the supplied diff and context. The added consts (AICRAFT_TEMPLATE_ID = "aicraft", base URL ending in /v1, non-empty default model, AICRAFT_API_KEY) satisfy every invariant the surrounding code and visible tests assert about fillable compatible rows: https host, Some(default_model), Some(api_key_env), id not shadowing an alias (zen/opencodezen/opencodego/sense-nova/base-ten/commandcode/cmd-code), and the row is placed so the partition-based settings_value() string is "opencode-zen, opencode-go, sensenova, baseten, groq, cerebras, command-code, aicraft; agnes unpublished", exactly what the updated assertion expects. The docs/PROVIDERS.md row is column- and order-consistent with the descriptor list and with the doc-comment list updated in the same commit. Open questions that this review could not resolve because the files are outside the supplied scope, and that should be checked before merging: (1) no build, test or lint run was performed here, so the assertion in the new tuple that ProviderKind::parse("aicraft") is None (and compatible_template_ids_do_not_shadow_built_ins) depends on crates/config/src/provider_kind.rs, which was not inspected; (2) whether any snapshot/assertion outside this module (e.g. in crates/tui/src/config.rs, crates/agent/src/lib.rs, or config.example.toml/docs/CONFIGURATION.md) hardcodes the previous settings_value() string, the template count, or the compatible-template id list, and would now fail; (3) whether scripts/check-provider-registry.py or the repository's docs-parity rule requires the same template row in docs/zh_hans/PROVIDERS.md, which this diff does not touch; (4) the credential_url for AICraft points at the site root rather than a key-management page as Groq/Baseten/Command Code do — a usability nit at most, not a defect, and not worth a finding on its own.
Advisory review by Codewhale (codewhale review --pr 6288 --post, head 34e8a5953fefc3c6c3140aae3b53839a4b1e2c7f). Line-specific findings are also posted as inline review comments; mechanical fixes arrive as committable suggestions you can apply from the Files tab. CODEOWNERS approval still governs merge.
…6289) Slice 1 of the template removal: hosted Chat Completions backends are ordinary named [providers.<name>] tables now, so the vendor list becomes documentation (base URLs, example models, key envs) instead of compiled rows. AICraft joins the table per #6171/#6288 with its advertised model families and no Anthropic default, per the issue. Gate: codewhale-config provider_templates 9 passed 0 failed (doc-drift test still green).
Lands @BX166's contribution from #6171. Their two commits are here verbatim,
with their authorship intact — this branch exists only because the fix below
could not be pushed to their fork.
What this adds
An
AICraft(aicraftapi.com) OpenAI-compatible provider setup template,following the descriptor-row pattern already used by SenseNova, Baseten, Groq,
Cerebras and Command Code: four consts plus one
ProviderSetupApply::Compatiblerow, inserted before the unpublishedagnesentry, with the two touched unit tests updated and the row documented in
docs/PROVIDERS.md.No new enum variant and no compiled model roster — live
GET /v1/modelsand theCodewhale catalog remain the offering list.
Why an integration branch
Their PR's required
Lintcheck was red onCheck formatting: onceaicraftjoins the tuple in
hosted_openai_compat_hosts_are_templates_not_enum_variantsthe line exceeds the width, so
cargo fmt --all -- --checkwants it wrapped.Six lines, entirely mechanical.
maintainerCanModifyis true on #6171, but pushing the fix toBX166/Codewhalereturned 404, so perAGENTS.md("when the push is refused,land the resolved merge on
integration/<topic>-<pr>-<date>and land fromthere") it lands here instead. Their branch is untouched.
Their CI had also never run at all: as a first-time contributor, every run sat
at
action_requiredfrom Sept 14 until approved today, and the approval has tobe repeated on each new commit. That is a repo setting, not anything they did.
Verification
rustfmt --edition 2024 --checkclean on the mergedcrates/config/src/provider_templates.rsmainfirst, so this carries today'srelease work rather than a five-day-old base
Closes #6171