fix(agents): review fixes — declared framework identity, strict flow key, pure variants_from#239
Merged
gewenyu99 merged 3 commits intoJul 15, 2026
Conversation
…key, pure variants_from Follow-ups from the review of #181: - skill-menu.json entries now carry `group` (hyphenated skill-id prefix), `framework` (the detection id a variant serves), and `default` (the variant a bare framework id resolves to when a family has several). Declared in the integration group's variant matrix and copied across by `variants_from`, so consumers resolve variants by exact match instead of reverse-engineering ids with alias tables and prefix guessing. Pairs with the wizard-side resolver change (PostHog/wizard PR on experiment/orchestrator-pi-runtask). - agent prompts must declare `flow:` — the build now rejects a missing key instead of only a contradicting one, since consumers filter prompts by it and a missing key built green here but silently dropped the prompt from the runtime registry. - `resolveVariantsFrom` is pure: it returns a resolved copy instead of mutating the config in place, which removes the `_variantsResolved` marker that existed to dodge the "variants or variants_from, not both" validation on re-expansion. Generated-By: PostHog Code Task-Id: fafc230d-6f14-4e4d-9462-0e7f18a1eec1
🧙 Wizard CIRun the Wizard CI and test your changes against wizard-workbench example apps by replying with a GitHub comment using one of the following commands: Test all apps:
Test all apps in a directory:
Test an individual app:
Show more apps
Results will be posted here when complete. |
Generated-By: PostHog Code Task-Id: fafc230d-6f14-4e4d-9462-0e7f18a1eec1
…plify) Generated-By: PostHog Code Task-Id: fafc230d-6f14-4e4d-9462-0e7f18a1eec1
gewenyu99
marked this pull request as ready for review
July 15, 2026 17:42
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-ups from the review of #181, targeting its branch. Pairs with PostHog/wizard#907: the
group/framework/defaultmenu fields added here are what that PR's exact-lookup variant resolver consumes (replacing the wizard'sFRAMEWORK_VARIANT_ALIASEStable andstartsWithguessing), so the two should land on their experiment branches together before the next joint run.What changed
Menu-declared framework identity. Each integration variant now declares
framework: <detection id>(the id the wizard's detector emits —rails,react-router, …) and, where a family has several variants, exactly onedefault: true.variants_fromcopies both across to the step-skills, andskill-menu.jsonentries gaingroup(hyphenated skill-id prefix),framework, anddefault. Consumers resolve a bare skill id + framework to a menu id by exact match; context-mill owns which variant serves which framework, so renames here can't silently strand a wizard-side alias table.Defaults chosen (parity with the current resolution order, flag anything you'd flip):
nextjs-app-router,nuxt-3-6,react-react-router-6,react-tanstack-router-code-based,react-native,astro-hybrid. Variants with no wizard detection id (react-vite,php,elixir,go,flutter) carry noframeworkand stay reachable by full id only.flow:is required. The build previously validated the key only when present, but consumers filter prompts by it — a forgottenflow:built green here and silently vanished from the runtime registry.assertFlowMatchesnow rejects a missing key with the folder's expected value in the message.resolveVariantsFromis pure. It returns a resolved copy instead of mutating the config in place. The_variantsResolvedmarker is gone — it existed to dodge the "declare either variants or variants_from, not both" validation on re-expansion, which is the tell that the mutation was the problem. The validation means what it says again, and repeated expansion is naturally stable (pinned by a new test).Testing
npm test— 14 files, 121 tests green (new: framework/default borrowing, input-config non-mutation, flow-key rejection viabuildAgents).npm run build—skill-menu.jsonemits 155 framework-tagged entries and 30 family defaults across the integration group and the four borrowed step-skill groups;agent-menu.jsonshape unchanged.Created with PostHog Code