Skip to content

fix(skills): narrow welcome kickoff routing#1591

Open
baixiaohang wants to merge 2 commits into
mainfrom
fix/welcome-kickoff-trigger
Open

fix(skills): narrow welcome kickoff routing#1591
baixiaohang wants to merge 2 commits into
mainfrom
fix/welcome-kickoff-trigger

Conversation

@baixiaohang

Copy link
Copy Markdown
Collaborator

Summary

  • restrict first-tree-welcome activation to the exact onboarding opener pairs or the explicit production-scan fix sentence
  • keep the OpenAI skill metadata and both generated AGENTS routing rows aligned with that contract
  • separate model-backed eval activation into auto-trigger, auto-ignore, and preloaded modes
  • add positive routing gates for admin, invitee, and greeting-free scan-fix, plus negative gates for ordinary chat and tree setup
  • bump first-tree-welcome to 1.2.2

Verification

  • python3 scripts/quick_validate_skill.py skills/first-tree-welcome
  • pnpm --filter @first-tree/client exec vitest run src/__tests__/agent-briefing.test.ts --maxWorkers=2 --minWorkers=1 (25 passed)
  • pnpm --filter @first-tree/skill-evals exec vitest run src/suites/first-tree-welcome/__tests__/floor.test.ts src/suites/first-tree-welcome/__tests__/grader.test.ts --maxWorkers=2 --minWorkers=1 (46 passed)
  • pnpm check
  • pnpm typecheck
  • git diff --check

Model-backed routing checks with gpt-5.6-sol:

  • admin exact opener: passed
  • invitee exact opener: passed
  • greeting-free scan-fix: passed
  • ordinary 你好: passed without reading the welcome skill, accessing repo/tree, or sending a tracked ask
  • tree-setup opener: passed without reading the welcome skill
  • representative preloaded periodic matrix case: passed

Known unrelated eval limitation

The existing readable-repo-populated-tree live case can still fail its exact evidence-snippet matcher even when the model reads both repo and tree evidence and emits three valid task options. This PR does not broaden into that non-routing matcher.

@baixiaohang
baixiaohang requested a review from yuezengwu as a code owner July 10, 2026 07:01

@yuezengwu yuezengwu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the routing contract and eval changes end to end. The welcome skill now activates only for the paired admin/invitee kickoff copy or the explicit scan-fix handoff, while ordinary and tree-setup chats remain negative routing cases. The SKILL.md frontmatter, OpenAI metadata, both generated briefing rows, and the preloaded/auto-trigger/auto-ignore eval split are consistent.

No blocker found. No database, schema, or core data-structure changes are involved.

Locally verified:

  • python3 scripts/quick_validate_skill.py skills/first-tree-welcome
  • client agent-briefing tests: 25 passed
  • welcome floor/grader tests: 46 passed
  • pnpm typecheck
  • pnpm check (passes; only existing unrelated warnings/info)
  • git diff --check

@yuezengwu yuezengwu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the synchronized head 6fdb3706.

The incremental change removes the remaining briefing drift risk by rendering both mutually exclusive First Tree family maps from one shared welcome-routing definition, and the new client test binds each rendered row to the shipped SKILL.md frontmatter. The implementation is safe: the constant is initialized before any exported briefing builder can call the helper, and both tree-less/tree-bound output paths are covered.

No code-level blocker found. No database, schema, or core data-structure changes are involved.

Locally verified on this head:

  • python3 scripts/quick_validate_skill.py skills/first-tree-welcome
  • client agent-briefing tests: 26 passed
  • welcome floor/grader tests: 46 passed
  • targeted Biome check
  • pnpm check (passes; only existing unrelated warnings/info)
  • pnpm typecheck
  • incremental git diff --check

One merge prerequisite remains: GitHub currently reports this PR as conflicting with main after #1590. When resolving packages/client/src/runtime/agent-briefing.ts, preserve #1590's readable-source / same-chat Phase 2 wording for both first-tree-seed rows, while keeping this PR's shared ${welcomeRow} rendering. The branch-side older seed wording should not win the conflict. The resolved head should be rechecked after synchronization.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants