feat(core): switch skills to progressive-disclosure model routing

[hqhq1025]

The bug

packages/providers/src/skill-injector.ts's matchSkillsToPrompt computed an intersection between trigger keywords extracted from the user prompt and from each skill's description. All four builtin skills (packages/core/src/skills/builtin/*.md) ship English-only descriptions, so a Chinese prompt could never produce an intersection, no matter how many Chinese aliases #88 added to the keyword table — both sides of the intersection had to share the same token. Result in production: skills: 0 in step=build_request.ok for every Chinese prompt, verified in ~/Library/Logs/@open-codesign/desktop/main.log.

The fix — progressive disclosure (level 1+2)

Same pattern Claude Design / Claude Code use. Three levels:

1. Always-load: skill name + description in system prompt at startup.
2. Model-routed: model reads the descriptions and decides relevance.
3. On-demand body load: lazy via tool round-trip.

For v0.x scope we collapse 1 and 3 into "always load all bodies". 4 builtin skills × ~1k tokens ≈ 4k tokens overhead per generation — trivial cost on Claude 4's 200k context, trivial implementation, no per-language gating, no keyword tables.

What changed

• Drop SKILL_TRIGGER_GROUPS, matchSkillsToPrompt, extractGroupIds from skill-injector.ts.
• Export formatSkillsForPrompt(skills) — no userPrompt argument, just canonical-sorted blobs.
• Rename collectMatchedSkillBlobs -> collectAllSkillBlobs. Add the success log step=load_skills.ok (only failure was logged before).
• composeSystemPrompt now wraps blobs under an explicit # Available Skills header that tells the model how to pick.
• Tests: removed the matching-behaviour assertions; added language-independent loading + empty-set + load-failure cases. Updated the step-order assertion in generate.test.ts for the new log.

Token-budget flag (stop-condition triggered)

Measured the new composeSystemPrompt({ mode: 'create', skills: blobs }) with all four builtin skills loaded:

• chars: 47,190
• approx tokens: ~11,798

Slightly over the ~10k flag in the brief. Not silently truncating — flagging here as the brief instructed. Two paths if it becomes a problem:

1. Strip code-fence examples from the longer skill bodies (mobile-mock has a 70-line iOS skeleton already duplicated in iosStarterTemplate).
2. Implement true level-3 disclosure (see follow-up below).

v0.2 follow-up

Implement level-3 progressive disclosure: keep only name + description in the always-loaded section, expose a loadSkill(name) tool, and let the model pull the body only when it commits to using a skill. That recovers the ~10k token budget and scales to many more skills.

PRINCIPLES checks

• Compatibility: ✅ no on-disk schema change, no IPC contract change.
• Upgradeability: ✅ blob format stable; v0.2 SkillTool slots in without rewriting composeSystemPrompt.
• No bloat: ✅ net -405 lines in this PR (deletes the keyword tables, simplifies the injector).
• Elegance: ✅ removes hand-curated bilingual lookup; matching is a model concern again.

Test plan

• pnpm typecheck clean
• pnpm lint clean (existing complexity warnings only)
• pnpm test all 132 vitest tests pass in core, 36 in providers
• Manual: pnpm --filter @open-codesign/desktop dev, send Chinese prompt, verify step=load_skills.ok and skills: 4 in ~/Library/Logs/@open-codesign/desktop/main.log

[github-actions]

Findings

• [Major] Disabled/provider-gated skills are no longer filtered before system-prompt injection — this can inject skills that explicitly set disable_model_invocation: true or provider restrictions, changing behavior and violating skill metadata contracts. Evidence packages/core/src/index.ts:389, packages/providers/src/skill-injector.ts:138.
  Suggested fix:

  // packages/core/src/index.ts
  import { filterActive, formatSkillsForPrompt } from '@open-codesign/providers';
  
  async function collectAllSkillBlobs(
    providerId: string,
    log: CoreLogger,
  ): Promise<{ blobs: string[]; warnings: string[] }> {
    // ...load skills...
    const active = filterActive(skills, providerId);
    const blobs = formatSkillsForPrompt(active);
    // ...
  }
  
  // in generate()
  const skillResult = input.systemPrompt
    ? { blobs: [], warnings: [] }
    : await collectAllSkillBlobs(input.model.provider, log);

Summary

• Review mode: initial
• 1 major issue found in the latest diff.
• docs/VISION.md and docs/PRINCIPLES.md were required by the workflow but are not present in this checkout (Not found in repo/docs).

Testing

• Not run (automation): pnpm is not available in this runner, so Vitest suites could not be executed.

open-codesign Bot

[github-actions]

[Major] formatSkillsForPrompt(skills) bypasses skill activation gating. Please filter with filterActive(skills, input.model.provider) before formatting, otherwise disable_model_invocation / provider-restricted skills can still be injected into the system prompt.

[hqhq1025]

Addressed Codex Major: collectAllSkillBlobs now reuses filterActive(skills, providerId) from packages/providers/src/skill-injector.ts before formatting. Skills with disable_model_invocation: true and skills whose trigger.providers excludes the current provider are dropped before the blob list is built. Provider id is plumbed through from input.model.provider.

Two new vitest cases in packages/core/src/generate.test.ts:

• drops skills with disable_model_invocation: true
• drops provider-restricted skills that do not match the current provider (and confirms the same skill is injected when the provider does match)

pnpm test 134/134 in core, full suite green; pnpm lint clean (only pre-existing complexity warnings). Commit 8676d36.

[github-actions]

Findings

• [Minor] Regression-prone test coverage drop for injectSkillsIntoMessages behavior — this PR removes most invariants (prefix scope routing, wildcard provider matching, insertion when no system message, immutability, multi-permutation determinism) while claiming behavior is unchanged; future refactors can now silently break these contracts. Evidence packages/providers/src/skill-injector.test.ts:32, packages/providers/src/skill-injector.test.ts:35.
  Suggested fix:

  // packages/providers/src/skill-injector.test.ts
  it('uses prefix scope to prepend to first user message', () => {
    const prefixSkill = makeSkill('prefix-skill', {
      frontmatter: {
        schemaVersion: 1,
        name: 'prefix-skill',
        description: 'Prefix skill.',
        trigger: { providers: ['*'], scope: 'prefix' },
        disable_model_invocation: false,
        user_invocable: true,
      },
    });
    const result = injectSkillsIntoMessages(BASE_MESSAGES, [prefixSkill], 'anthropic');
    expect(result[0]?.content).toBe('You are a helpful assistant.');
    expect(result[1]?.role).toBe('user');
    expect(result[1]?.content).toContain('Body of prefix-skill.');
  });
  
  it('inserts a system message when no system message exists', () => {
    const messages: ChatMessage[] = [{ role: 'user', content: 'Design a landing page.' }];
    const result = injectSkillsIntoMessages(messages, [makeSkill('test-skill')], 'anthropic');
    expect(result[0]?.role).toBe('system');
  });

Summary

• Review mode: follow-up after new commits
• 1 minor issue found in the latest diff.
• docs/VISION.md and docs/PRINCIPLES.md were required context but are unavailable in this checkout (Not found in repo/docs).

Testing

• Not run (automation)

open-codesign Bot

[github-actions]

injectSkillsIntoMessages() is marked as "unchanged behaviour" here, but this diff drops most of its contract tests (prefix scope, no-system insertion, wildcard matching, immutability, permutation stability). Please restore a minimal set of those invariants so future changes can’t regress this path silently.

[hqhq1025]

Addressed Codex [Minor]: restored explicit injectSkillsIntoMessages coverage in 1345268. Added 6 new test cases under a new surviving behaviour describe block — empty skills, skill block ordering before existing system content, new system message insertion, provider filter drop, disable_model_invocation drop, and multi-skill blob inclusion. Did not re-introduce the deleted keyword-matcher tests (intentionally removed). Full pnpm test and pnpm lint green.

[github-actions]

Findings

• [Major] Breaking public export classified as minor release — @open-codesign/providers removes matchSkillsToPrompt from its public surface, which is a source-breaking change for consumers importing that symbol; the changeset still marks this package as minor, so downstream updates can break unexpectedly. Evidence packages/providers/src/index.ts:235, .changeset/skills-progressive-disclosure.md:3.
  Suggested fix:

  ---
  '@open-codesign/core': minor
  '@open-codesign/providers': major
  ---

Summary

• Review mode: follow-up after new commits
• 1 major issue found in the latest diff.
• docs/VISION.md and docs/PRINCIPLES.md: Not found in repo/docs.

Testing

• Not run (automation)

open-codesign Bot

[github-actions]

matchSkillsToPrompt was removed from the public exports here, but the changeset marks @open-codesign/providers as minor. This is a source-breaking API change for external imports; please either keep a compatibility export for one cycle or bump this package to major in the changeset.

[hqhq1025]

Addressed Codex [Major]: existing .changeset/skills-progressive-disclosure.md now explicitly calls out the BREAKING removal of matchSkillsToPrompt and SKILL_TRIGGER_GROUPS from @open-codesign/providers's public surface. Kept it as a minor bump per Changesets pre-1.0 semver (Option A); desktop is the only consumer and was already migrated in this PR. Pushed as 57b61ae. Lint + tests green.

[github-actions]

Findings

• None.

Summary

• Review mode: follow-up after new commits
• No high-confidence issues found in added/modified lines of the latest PR diff.
• docs/VISION.md and docs/PRINCIPLES.md: Not found in repo/docs.
• Residual risk: no E2E coverage in this diff proving the Chinese-prompt path in desktop wiring; current coverage is unit-level.

Testing

• Not run (automation)

open-codesign Bot