Skip to content

feat(01KVS12K): A skill that helped a story reach done is credited as useful even when no READY-FOR-MERGE verdict event was recorded#438

Merged
jackmcintyre merged 2 commits into
mainfrom
story/native-01kvs12kceyjtvpn84nmraptt4-a-skill-that-helped-a-story-reach-done-i
Jun 23, 2026
Merged

feat(01KVS12K): A skill that helped a story reach done is credited as useful even when no READY-FOR-MERGE verdict event was recorded#438
jackmcintyre merged 2 commits into
mainfrom
story/native-01kvs12kceyjtvpn84nmraptt4-a-skill-that-helped-a-story-reach-done-i

Conversation

@jackmcintyre

Copy link
Copy Markdown
Owner

What changed

A skill that helped a story reach done is credited as useful even when no READY-FOR-MERGE verdict event was recorded

This change delivers the following acceptance criteria:

  • AC1: Given a skill.invoke whose story_id reached done/ (its manifest is in .flow/state/done/) but for which NO joined later READY-FOR-MERGE reviewer.verdict event exists in telemetry, When computeSkillEffectiveness runs, Then that invocation is counted as a useful fire (useful_fire_count includes it and effectiveness_ratio is above 0) via a declared done/-manifest read, rather than scoring 0.
  • AC2: Given a skill.invoke whose story did NOT reach done/ and has no joined READY-FOR-MERGE verdict, When effectiveness is computed, Then it is NOT credited (a genuinely unhelpful skill still scores 0); and a skill.invoke that already joins a later READY-FOR-MERGE reviewer.verdict still counts as a useful fire exactly as today.

Why

As a maintainer relying on the retro to keep the skill set healthy, I want skill-effectiveness to credit a skill.invoke as a useful fire when its joined story reached done/, not only when a later READY-FOR-MERGE reviewer.verdict event exists in telemetry, so that the retro does not draft a false skill-retire/revise proposal against a skill that helped a story complete through a path that recorded no READY-FOR-MERGE verdict event (for example an auto-merge-gate or operator-merge completion).

How to check it yourself

  1. Run vitest on plugins/flow/mcp-server/src/tools/tests/skill-effectiveness-attribution.int.test.ts — the AC1 test verifies a done-manifest-only invoke is credited (useful_fire_count: 1, effectiveness_ratio: 1). 2. Run vitest on plugins/flow/mcp-server/src/tools/tests/compute-skill-effectiveness.test.ts — the done/-manifest suite (AC1/AC2a/AC2b/ENOENT) verifies credit vs no-credit vs ENOENT posture via injected seams. 3. Confirm no existing tests regressed — all 24 tests in both files must pass.

Risk and blast radius

Risk tier: medium

The change is scoped to the files in the diff below. Review the diff to judge its actual blast radius — including any effect on shared state, data schemas, or authentication paths — before approving.

What is explicitly not covered: reviewer verification of this summary's accuracy is handled by a separate companion story.

Evidence

The pre-pull-request build-and-test gate passed before this pull request was opened. No pull request can be opened by the automated flow unless both pnpm build and pnpm test exit 0 in the developer's working directory.

Per-criterion covering checks:

  • AC1 → plugins/flow/mcp-server/src/tools/__tests__/skill-effectiveness-attribution.int.test.ts (automated test)
  • AC2 → plugins/flow/mcp-server/src/tools/__tests__/compute-skill-effectiveness.test.ts (automated test)

Story: native:01KVS12KCEYJTVPN84NMRAPTT4
Spec: .flow/native-stories/01KVS12KCEYJTVPN84NMRAPTT4.md
ACs:

  • AC1: Given a skill.invoke whose story_id reached done/ (its manifest is in .flow/state/done/) but for which NO joined later READY-FOR-MERGE reviewer.verdict event exists in telemetry, When computeSkillEffectiveness runs, Then that invocation is counted as a useful fire (useful_fire_count includes it and effectiveness_ratio is above 0) via a declared done/-manifest read, rather than scoring 0.
  • AC2: Given a skill.invoke whose story did NOT reach done/ and has no joined READY-FOR-MERGE verdict, When effectiveness is computed, Then it is NOT credited (a genuinely unhelpful skill still scores 0); and a skill.invoke that already joins a later READY-FOR-MERGE reviewer.verdict still counts as a useful fire exactly as today.

Credit skill.invoke as useful when its story reached done/ with no READY-FOR-MERGE verdict event

jackmcintyre and others added 2 commits June 24, 2026 07:32
… useful even when no READY-FOR-MERGE verdict event was recorded

Adds a done/-manifest read to computeSkillEffectiveness so that a
skill.invoke whose story reached done/ is credited as a useful fire even
when no READY-FOR-MERGE reviewer.verdict event was recorded in telemetry
(e.g. completion via the auto-merge gate or operator merge on the
paused-for-human path). The fix augments — not replaces — the existing
verdict join: the done/ directory is read as a new declared injected
dependency (readDoneRefsImpl), and any invoke whose story_id is found in
done/ is credited. The integration test AC1 uses a fresh isolated temp
dir to avoid the two-in-progress-manifest ambiguity that prevented
story_id attribution; both AC1 and AC2 pass with the fixed
implementation.
@jackmcintyre jackmcintyre merged commit 16c12b5 into main Jun 23, 2026
@jackmcintyre jackmcintyre deleted the story/native-01kvs12kceyjtvpn84nmraptt4-a-skill-that-helped-a-story-reach-done-i branch June 23, 2026 21:39
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.

1 participant