fix(build): look up example skip patterns by directory name - #285
Open
Ref34t wants to merge 1 commit into
Open
Conversation
Ref34t
force-pushed
the
fix-example-skip-pattern-lookup
branch
from
July 28, 2026 09:41
b3fd888 to
f9ede84
Compare
Keys in the examples: block of skip-patterns.yaml are example directory names, but a skill with a single example path looked its overrides up by skill.id. The lookup missed, mergeSkipPatterns read it as no overrides, and the build carried on with no error, so rules that were written on purpose never ran. Extract the lookup so it always keys on the directory name and can be tested. isSingle still selects the output filename and display title, which the build generates; config keys are typed by hand and are always directories. Rebuilt: integration-laravel drops 20,896 bytes (bootstrap/cache/services.php, Laravel's generated package cache) and integration-android drops 16,141. integration-swift already took the dirName branch and is unchanged. Fixes PostHog#284
Ref34t
force-pushed
the
fix-example-skip-pattern-lookup
branch
from
July 28, 2026 09:46
f9ede84 to
2c2b74a
Compare
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.
Fixes #284.
Problem
Per-example overrides in
context/skip-patterns.yamlwere looked up by the wrong key for any skill with a single example, so they never applied.scripts/lib/skill-generator.js:644Keys in the
examples:block are example directory names —laravel,android,swift-xcodegen. With one example path the lookup usedskill.id(integration-laravel) instead, missed, and returnedundefined.mergeSkipPatternstreats that as "no overrides" and the build carried on with no error.isSingleis the right switch for the other two things it controls — the output filename and the display title, both generated by the build. Config keys are typed by hand, and they are always directory names, so the same branch shouldn't apply here.Fix
One lookup, always by directory name, extracted so it can be tested:
No key in
skip-patterns.yamlis a skill id today, so nothing that currently resolves stops resolving.Effect on shipped skills
Two skills had overrides that were being dropped. Rebuilt and compared
references/EXAMPLE.md:integration-laravelintegration-androidThe Laravel case is almost entirely
bootstrap/cache/services.php, Laravel's generated package-discovery cache — a machine-written list of class names that Laravel's own.gitignoreexcludes. It was 17% of that skill. For scale,references/COMMANDMENTS.mdin the same bundle is 1,618 bytes.integration-swifthas two example paths, so it was already taking thedirNamebranch. Itsallowrescue forproject.ymlstill works after the change — verified, still 8 occurrences inEXAMPLE-swift-xcodegen.md.Tests
Five added to
scripts/lib/tests/skip-patterns.test.js, including the one that pins the bug:The existing suite covered
mergeSkipPatternsandshouldSkip, which were both correct — the bug was in the key handed to them, which nothing exercised.Worth a follow-up
The lookup failed silently. A key in
examples:that matches no example directory produces no warning, so a rule can look right in the file and do nothing. A build-time check that everyexamples:key resolves would turn this class of bug into an error. Left out here to keep the change to one thing — happy to add it if you want it.Separately,
bootstrap/cache/*.phpis generated output committed toexample-apps/laravel. This PR stops it reaching the bundle, but removing it from the repo would be worth doing on its own.