Repository navigation
Conversation
The eight omnibus instrument skills carried the same three bullets for finding the project token and host. Move them into the context/shared/project-token-and-host.md partial and include it in each skill, so the next change is one edit and the copies cannot drift. expandPartials now indents every included line with the directive's leading whitespace, so an indented include nests under its list item. Column-0 includes build as before. Built skills are byte-identical except instrument-tracing, where the phc_/phx_ bullet now follows the host bullet. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
🧙 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
Test against a wizard branch:
Add Results will be posted here when complete. |
Contributor
There was a problem hiding this comment.
PostHog Review · Standard · reviewed 73f4f01: 1 Should fix.
See this review and its findings in PostHog Review. Comment @posthog review --deep for a Deep review. A Deep review takes longer and looks deeper than a Standard review.
The dev server watched only context/skills, context/agents and example-apps, and routeChange ignored context/shared. Editing the project-token-and-host partial during `npm run dev` left the eight instrument skills stale until a restart, where editing the bullets in place rebuilt them before. Watch context/shared and route a change there to every skill. Any skill may include a partial, and the router does no I/O to find which do. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Replace the inline comment with an everySkillId helper, and say in the dev-server header that an edit under context/shared/ rebuilds every skill. The router does no I/O, so it cannot tell which skills include a given partial; rebuilding them all took about 3 s in a live run. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
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.
Problem
The eight
omnibus/instrument-*skills each carried the same three bullets, word for word, telling the agent how to find the PostHog project token and host. #511 had to make the same edit in all eight files, and the copies could drift apart on the next change.Changes
context/shared/project-token-and-host.mdholds the three bullets. Each of the eight skills now includes it with{{> project-token-and-host}}, so the next change to this guidance is one edit.expandPartialsindents every included line with the directive's leading whitespace. An indented include now nests under its list item instead of jumping to column 0. Column-0 includes, such as the existingmcp-tool-callingones, build exactly as before.instrument-tracing, thephc_/phx_warning bullet sat between the token and host bullets. It now comes right after the include.npm run devnow watchescontext/shared/and rebuilds every skill when a partial changes. Before, it ignored that directory, so after this extraction an edit to the shared bullets would have left the eight skills stale until a restart. The router can't see which skills include which partial, so it rebuilds them all, which took about 3 seconds in a live run.mcp-analyticswords this guidance differently and is left alone.instrument-reverse-proxy, which landed on main after this branch started, has no copy of these bullets.Stacked on #511, so it shares that PR's final bullet text. Retarget to
mainonce #511 merges.Note
🔵 Merge risk: low · two-way door
omnibus/instrument-*skill gets the same skill text as on the base, in both regions. A build of every skill before and after, unzipped and diffed, shows no change except the build timestamp and the moved bullet below.instrument-tracingagents see thephc_/phx_warning one bullet later, after the host bullet instead of before it. The wording is unchanged.expandPartialsserves every skill that uses a partial. Column-0 includes, which are all the existing ones, are covered by the same build diff and a new unit test.npm run devnow get a rebuild of every skill when a file undercontext/shared/changes, where before nothing rebuilt. A live run covers it; the released build doesn't use this code.How did you test this code?
generateSkill: a new test inscripts/lib/tests/partial-includes.test.jsbuilds a skill whose indented{{> ...}}sits inside a nested list and checks theSKILL.mdit writes. It failed on the base (the bullets landed at column 0) and passes now. A second test pins column-0 includes as unchanged.vitest run scripts: every test passes.context/shared/failed on the first commit (the router returned null) and passes on the fix.npm run dev: edited the shared partial and fetchedomnibus-instrument-logs.zipfrom the running server. On the first commit, nothing rebuilt and the zip kept the old text. On the fix, every skill rebuilt and the zip carried the edit, nested under its list item.node scripts/build.json the base and on HEAD, everydist/skills/*.zipunzipped,diff -rover the trees. The only differences are the manifest'sbuildTimestampand the movedphx_line inomnibus-instrument-tracing/SKILL.mdand its two marketplace copies. Rerun on the final head with the same result.🤖 Generated with Claude Code