fix(curator): verify consolidation artifacts - #18393
Closed
teknium1 wants to merge 2 commits into
Closed
Conversation
The consolidation-artifact verifier added in the salvage of #18225 requires the umbrella's support file to exist on disk to classify a removed skill as 'consolidated'. test_curator_rewrites_cron_skills_when_skill_consolidated (added in #18253 after this PR was authored) used a tool-call-only fixture that the tightened classifier now correctly rejects as a hollow umbrella. Write the real file so the test exercises the cron-rewrite path it is actually trying to cover.
alt-glitch
added
type/bug
Contributor
Author
|
Closing — treating the root cause as a model problem, not a curator-classification problem. If the LLM writes a hollow umbrella SKILL.md with TOC links it never fills in, the fix belongs upstream of the classifier (prompt / model selection / umbrella-first prompt constraints), not in a downstream verifier that papers over the bad behavior. Thanks @helix4u — will revisit if we keep seeing hollow umbrellas after prompt-side work. |
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
3 participants
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.
Salvage of #18225 (@helix4u) — rebased onto current main with a fixture fix.
Summary
Curator no longer marks a removed skill as successfully "consolidated" when the umbrella is a hollow stub whose referenced support files were never actually written.
What changed
agent/curator.py— classification now records evidence kind (support_file_write,support_link,content_reference) and verifies the artifact exists on disk before counting it as a consolidation. Addsincomplete,incomplete_reasons,unsafe_consolidations,structured_summarytorun.jsonand surfaces incomplete-run warnings inREPORT.md.tests/agent/test_curator_classification.py+test_curator_reports.py— regression tests for hollow umbrella references and valid support-file consolidations.tests/agent/test_curator_reports.py(follow-up) — test_curator_rewrites_cron_skills_when_skill_consolidated (added on main in fix(curator): rewrite cron job skill refs after consolidation #18253 after this PR was authored) used tool-call-only evidence that the tightened classifier correctly rejects. Fixture now materializes the umbrella's support file on disk so the test exercises the cron-rewrite path it's actually covering.Validation
Context
Addresses the class of curator bug behind issue #18373 ("Curator should not auto-archive user-created custom skills without dry-run/approval") — if a curator run produces hollow umbrellas and then archives the originals, the originals were effectively lost. With this verifier, those archives stop being classified as consolidations, so the umbrella-first prompt's destructive path is gated on real evidence.
Closes #18225.
Co-authored-by: helix4u 4317663+helix4u@users.noreply.github.com