Skip to content

fix(skills): require use evidence for patch review hints - #4713

Closed
ScriptedAlchemy wants to merge 2 commits into
masterfrom
fix/skill-review-missing-use-evidence
Closed

ScriptedAlchemy wants to merge 2 commits into
masterfrom
fix/skill-review-missing-use-evidence

Conversation

@ScriptedAlchemy

@ScriptedAlchemy ScriptedAlchemy commented Nov 3, 2026 •

Copy link
Copy Markdown
Owner

Problem

The usage recommender treats use_count <= patch_count as evidence for a repair hint after two patches. With unmeasured usage, the default zero satisfies that comparison, so merely patching a skill twice manufactures a repair recommendation. The stale-skill path already treats missing usage as unknown, but the improvement path did not.

Change

Require a recorded use count and timestamp before applying the existing observed-use patch-review heuristic. Missing or incomplete use evidence produces the existing non-actionable recommendation with an explicit evidence-gap reason, not a claim that no skill work is needed. Raw patch/view/use evidence remains available.

Pinned, user-authored, archived and disabled exclusions still run first. The observed-use policy, schemas, storage, MCP/dashboard response shape, writer execution, manual authoring and mutation authority are unchanged. The existing writer consumes this shared recommendation API, so missing telemetry no longer creates its managed_skill_patch hint.

Regression coverage

Updated the previous unmeasured-usage fixture to assert no repair hint while retaining all evidence. Added tests for incomplete count/timestamp pairs, the existing policy with observed use, and pinned/user-authored/disabled/archived exclusions with observed usage. The existing persisted-ledger integration test records a real Use event before its two patches and retains its positive expectation unchanged.

Validation

Reviewed the actual recommender, its writer consumer, MCP/dashboard call sites, existing persisted-ledger test, and the complete GitHub-generated commit diff. The diff changes only this recommender and its unit tests.

Rust compilation, test execution, rustfmt and Clippy have not run locally: this environment has no supplied checkout or Rust/Bazel toolchain. No Cargo commands were run. Hosted CI must execute the automation-runtime unit tests and the existing agent-suite skill-usage journey before merge. No model-quality or cost-saving measurement is claimed.

Refs #4538. This fixes the missing-telemetry inference only. Kev qualification/shadow evaluation, the separate keyword diagnosis, and deciding whether to retire the remaining observed-use heuristic remain open.


Devin Review

@changeset-bot

changeset-bot Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 18c71bb

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@devin-ai-integration devin-ai-integration Bot 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.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Devin Review

@ScriptedAlchemy

Copy link
Copy Markdown
Owner Author

Consolidated into #4742 (integrate/open-pull-requests) at the captured head shown in its description, as requested. Continue review and further fixes on #4742. This closure does not assert CI readiness; the replacement remains draft. Source branch retained.

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