Repository navigation
feat(aws-startup-advisor): promote google and openrouter to supported with shared mapping guides - #469
Conversation
… in the AI capability catalog Follow-up to aws#450. That PR deliberately marked `google` and `openrouter` as `unknown` in ai-capability-catalog.json because neither had a shared, cross-skill Bedrock-mapping guide under skills/shared/ai/ — the bar the catalog uses for `supported`. Gemini's only guide lived under gcp-to-aws's adapter-local design-refs/, and no gateway-routing guide existed at all. This PR delivers those two shared guides and flips both catalog entries: - skills/shared/ai/ai-gemini-to-bedrock.md — source-cloud-neutral Gemini → Bedrock model-selection guide, modeled on the shared OpenAI/Anthropic guides (same header/vendoring contract). The Bedrock target depends on the Gemini model, not the source cloud, so the guide is canonical here and vendored into gcp-to-aws and azure-to-aws. - skills/shared/ai/ai-openrouter-to-bedrock.md — gateway-routing guide that maps each OpenRouter-fronted upstream model to Bedrock via the per-provider guides, preserving per-upstream associations and BYOK-vs-credits cost provenance. OpenRouter stays a gateway, never a provider or infrastructure source. - catalog: google -> supported, openrouter -> supported, catalog_version 1 -> 2, verified_at refreshed, notes rewritten to cite the new guides. Target-only, matching aws#450/aws#415/aws#416: no phase file reads these; no runtime behavior changes; implemented_adapter/route stay null everywhere (status reflects documentation coverage, not an implemented adapter). The gcp-to-aws adapter-local design-refs/ai-gemini-to-bedrock.md is retained; repointing its runtime routing onto the shared guide is deferred to the extraction stages (aws#389 3a-3e). Gates: vendored-parity (55 files byte-identical), skill-refs (1357 refs, 0 missing), ai-capability-catalog contract tests (13), full plugin suite (444), lint:md (803 files, 0 errors), artifacts/consent-disclosure/model-id all pass.
…pter-local drift Addresses a review flag on this PR: gcp-to-aws now holds two Gemini → Bedrock guides — the new inert shared/vendored copy and the older adapter-local references/design-refs/ one that design-ai.md still loads at runtime — with no gate between them. The vendored-parity check only compares the vendored copy against the shared canonical, never against the adapter-local original, so a one-sided pricing edit could let them silently disagree until the aws#389 3a-3e extraction folds them together. - Make the shared guide's priced body byte-identical to the adapter-local guide's (the two had drifted by a single cosmetic table-separator character; all pricing/mapping rows already matched). Headers stay intentionally different (canonical banner + source-neutral Applies-to). - Add tests/test_gemini_guide_body_parity.py: pins both guides' bodies (from the first `## ` heading down) byte-for-byte, excluding the lead-in. Verified it fails on a one-sided pricing mutation. Delete this test when the extraction stage merges the two guides into one. No runtime routing changes; still target-only. Full `mise run ci` passes (446 tests, +2).
|
Thanks for the review — addressed all three. 1. Two Gemini guides with no drift check — fixed in-PR (
This turns the deferred drift risk into an enforced gate without touching runtime routing. 2. 3. Net change: +2 tests (446 total), three Gemini-guide files touched to make bodies byte-identical; full |
leon1418
left a comment
leon1418
left a comment
There was a problem hiding this comment.
[🤖 AI review 🤖]
Requesting changes at 9a93a8a7c6d1a2124919857234f16f78b4031b1d for three inconsistencies in the new OpenRouter guide:
- Match the captured provenance states and fields; keep uncertainty handling and avoid inventing a mixed token split.
- Preserve measured OpenRouter spend as the customer baseline.
- Carry the provider-selected GPT endpoint and matching credentials into cutover.
Each correction applies to skills/shared/ai/ai-openrouter-to-bedrock.md and its GCP/Azure vendored copies. These findings concern the new reference contract; the guides are intentionally not wired into a runtime phase.
Reviewed all 13 changed files with OCR-assisted repository review and one independent general review. Local validation passed: 446 plugin tests, 480 agent-advisor tests, 301 llm-to-bedrock tests, both TypeScript suites (13 + 44), Markdownlint, artifact/reference/model/consent checks, vendored parity, root validation, and git diff --check. Current GitHub checks are green. No live migration was exercised.
…er/Estimate contracts Addresses three P2 review findings on the ai-openrouter-to-bedrock.md guide (canonical + both vendored copies): 1. Provenance enum: the guide used invented `byok`/`credits` values and told the reader to derive a BYOK/credits volume split. The producer (discover-openrouter-api.md) actually emits `byok_passthrough`/`no_byok`/`mixed`/`uncertain`/`unknown` with a single `monthly_cost_usd` and no funding/token ratio, and llm-to-bedrock/SKILL.md forbids deriving that split. Rewrote the table to the real enum, added the `uncertain` handling, and state the mixed split is unavailable rather than fabricating one. 2. Cost baseline: the guide told the reader to compare the Bedrock target against the upstream provider's list price instead of the captured OpenRouter spend, which would swap the savings denominator. estimate-ai.md Part 1 prefers measured current_costs/usage-profile dollars and does not fall back to a direct-provider list rate for the OpenRouter portion (no source-side list row in pricing-cache.md). Made the captured spend the baseline and demoted list pricing to a separately labeled reference. 3. Endpoint/migration_path: the Cutover step sent every GPT target to Mantle, but ai-openai-to-bedrock.md can select `runtime_openai_cris` (CRIS) for GPT-5.6/GPT-6. Now carries each target's selected migration_path, endpoint/ profile ID, and matching IAM (bedrock:InvokeModel for Converse/runtime, bedrock-mantle:* for the Mantle GPT path) through cutover. Docs-only, target-only; vendored parity, skill-refs, lint:md green; 446 tests pass.
leon1418
left a comment
leon1418
left a comment
There was a problem hiding this comment.
[🤖 AI review 🤖]
Reviewed 643e68d4747d29335d0721edc9fe339682dc7d94 against base bc39e268867d23795eb1aa222eda1f510502a50b with OCR-assisted repository review and a fresh independent general review. No remaining substantive findings. The 13-file PR patch is unchanged from the last verified version; the relevant base changes preserve its contracts.
Local validation passed: 467 plugin tests, 509 agent-advisor tests, 301 llm-to-bedrock tests, both TypeScript suites (13 + 44), registered linters, reference/artifact/vendor checks, root validation, and git diff --check. Current GitHub checks are green. No live migration was exercised.
The earlier fixes remain intact and the three original threads remain resolved. Approval is recommended for the user to submit.
Problem
The AI source-provider capability catalog added in #450 marks two entries
"unknown"even though the plugin already works with both: the direct AI providergoogle(Gemini) and the gatewayopenrouter. The catalog's bar for"supported"is a shared, cross-skill Bedrock-mapping guide underskills/shared/ai/, and neither cleared it — Gemini's only guide lived undergcp-to-aws's adapter-localreferences/design-refs/, and OpenRouter had no mapping guide of any kind, only runtime detection indiscover-openrouter-api.md.This leaves the catalog understating what the migration flow can map, and leaves a future shared AI engine (issue #389's extraction) without a source-neutral Gemini guide or any gateway-routing guide to build against.
Fix
Add the two missing shared mapping guides, then flip both catalog entries to
"supported".ai-gemini-to-bedrock.mdis a source-cloud-neutral Gemini → Bedrock model-selection guide carrying the model-mapping tables, pricing comparisons, and lifecycle checks, modeled on the shared OpenAI and Anthropic guides with the same header and vendoring contract. The Bedrock target depends on the Gemini model, not on which cloud served it, so the guide is canonical underskills/shared/ai/and vendored byte-identically intogcp-to-awsandazure-to-aws.ai-openrouter-to-bedrock.mdis a gateway-routing guide: it maps each OpenRouter-fronted upstream model to Bedrock through the per-provider guides, preserves per-upstream associations (provider → family → exact model) and BYOK-vs-credits cost provenance, and keeps OpenRouter a gateway rather than a provider or infrastructure source.With the guides in place,
googleandopenrouterboth move from"unknown"to"supported",catalog_versiongoes 1 → 2,verified_atis refreshed, and both notes are rewritten to cite the new guides.Scope
This change affects only the
aws-startup-advisorplugin's shared AI reference set: the two new shared guides, their vendored copies undergcp-to-awsandazure-to-aws, the capability-catalog data file (canonical plus both vendored copies), and the three README tables that index them.It is target-only, matching #450/#415/#416.
status: "supported"means a shared mapping guide exists; it is not a claim that an adapter is wired up.implemented_adapter(providers) androute(gateways) remainnullon every entry, and no phase file reads these guides. No runtime behavior, question text, model mapping, pricing value, or artifact name changes.Repointing runtime routing onto the shared Gemini guide is out of scope. The
gcp-to-awsadapter-localreferences/design-refs/ai-gemini-to-bedrock.mdand its Discover/Design routing are left untouched; moving them onto the shared guide is deferred to the extraction stages (#389 steps 3a–3e). An implemented routing adapter for OpenRouter is likewise a separate, stricter bar tracked byroute, which staysnull.This PR is self-contained and does not require another PR to land first. It builds directly on #450 (merged).
In code, for reviewers
skills/shared/ai/ai-gemini-to-bedrock.md(new) neutralizes the GCP/Vertex framing of the adapter-local guide (drops thegcp-to-aws-localreferences/shared/pricing-cache.mdreference, keeps only Bedrock-side vendored references) so its paths resolve from any consuming skill under the skill-refs gate.skills/shared/ai/ai-openrouter-to-bedrock.md(new) defers per-model selection toai-anthropic-to-bedrock.md/ai-openai-to-bedrock.md/ai-gemini-to-bedrock.mdand documents the gateway cutover shape (transport, credentials, routing policy, per-upstream validation).skills/shared/ai/ai-capability-catalog.jsonflipsgoogleandopenroutertosupported, bumpscatalog_versionandverified_at, and rewrites both notes; vendored copies ingcp-to-awsandazure-to-awsare synced byte-identical viatools/sync-vendored.py.skills/shared/ai/README.mdadds both guides to the canonical table and updates the "Deliberately NOT here" note; bothreferences/vendored/README.mdtables gain the two rows the vendored-parity gate requires.Validation
Validation recorded for current head
8f6bf029370b18d41eabbce3569a06ff8d6728f9:mise -C plugins/aws-startup-advisor run cipassed.test_ai_capability_catalog_contracts.pytests (schema validity, semantic invariants, vendored byte-identity, negative mutations) stay green against the flipped catalog.lint:vendored-parity— 55 vendored files byte-identical toskills/shared/(+4 vs. baseline).lint:skill-refs— 1357 references checked, 0 missing.python3 tools/validate.py(repo root) — all validations passed.The tests validate the catalog contract, vendored parity, and reference resolution against the committed fixtures; they do not exercise a live migration run. Consistent with #450, the guides are not wired into any phase and
implemented_adapter/routeremainnull, so there is no runtime path to exercise here.Type of Change
This adds two shared AI mapping guides and promotes two existing capability-catalog entries to
supported, with no runtime wiring.Startup Advisor review checklist
By submitting this PR, I confirm that:
mise -C plugins/aws-startup-advisor run ciand all checks pass.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.