Skip to content

fix: add write-hygiene guardrails to skill review prompts (prevent SKILL.md bloat) - #23288

Open
gejifeng wants to merge 1 commit into
NousResearch:mainfrom
gejifeng:fix/skill-review-completion-bias
Open

fix: add write-hygiene guardrails to skill review prompts (prevent SKILL.md bloat)#23288
gejifeng wants to merge 1 commit into
NousResearch:mainfrom
gejifeng:fix/skill-review-completion-bias

Conversation

@gejifeng

@gejifeng gejifeng commented May 10, 2026

Copy link
Copy Markdown

Problem

The background skill-review agent continuously inflates SKILL.md files across sessions. Root causes:

  1. Completion bias_SKILL_REVIEW_PROMPT said "most sessions produce at least one skill update", so the model treats no-op as a failure.
  2. No read-before-write requirement — the agent never calls skill_view first, so it cannot detect duplicate content.
  3. Date stamps — the model self-injects temporal markers (Updated 2026-05-10, NEW — Session ...) that accumulate across sessions.

Related upstream discussions: #12877, #20595, #22620.


Fix — two-layer defence

Layer 1 — Prompt-level (run_agent.py)

In both _SKILL_REVIEW_PROMPT and _COMBINED_REVIEW_PROMPT:

  • Removed "Be ACTIVE — most sessions produce at least one skill update" (the completion-bias trigger).
  • Added WRITE HYGIENE rules: no date stamps, no duplicate sections, read before write, prefer patch over edit, treat nothing to save as the correct default.

Layer 2 — Code-level hard rejection (tools/skill_manager_tool.py)

Added _run_write_hygiene() called by _edit_skill() and _patch_skill() before any bytes hit disk. It contains three independent hard-rejection checks that return tool-call errors, forcing the model to fix the content rather than ignoring prompt instructions:

Check What it catches Where
_check_date_stamps() Updated 2026-05-10, Added 2024-11, <!-- session … --> etc. Both edit and patch
_check_duplicate_headings() Same ## heading appears twice in new content Both edit and patch
_check_content_growth() Single write more than doubles file size (>100% + >500 chars) edit only

The growth guard is disabled for patch (targeted by design); date-stamp and duplicate-heading checks apply to both.


Why prompt-only is insufficient

As noted in #20595: "Completion bias is a cognitive failure mode, not a prompt-attention failure." The model will comply with soft wording when convenient and ignore it when under pressure to produce output. Code-level checks are the only way to guarantee enforcement.


Testing

# All pass:
assert _check_date_stamps("Updated 2026-05-10 rule")   # blocked
assert _check_date_stamps("## Style Rules") is None      # allowed
assert _check_date_stamps("<!-- session 2025-03-01 -->") # blocked
assert _check_date_stamps("Version 3.2.1 info") is None  # allowed
assert _check_duplicate_headings("## Foo\n\n## Foo\n")   # blocked
assert _check_duplicate_headings("## Foo\n\n## Bar\n") is None  # allowed
assert _check_content_growth("x" * 2001, "x" * 1000)    # blocked (200%)
assert _check_content_growth("x" * 1800, "x" * 1000) is None  # allowed (180%)

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint tool/skills Skills system (list, view, manage) labels May 10, 2026

@teknium1 teknium1 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.

Thank you for identifying the completion-bias issue; it remains present on current main in agent/background_review.py:171-175 and :282-284.

Problems

  • The prompt source moved: run_agent.py:1576-1582 imports these constants from agent.background_review, so the run_agent.py portion needs to be ported rather than applied directly.
  • The proposed hard checks run for all create/edit/patch callers. Current main deliberately distinguishes autonomous review from foreground user-directed edits in tools/skill_manager_tool.py:297-312; please avoid applying heuristic date/heading/growth rejections to foreground writes.
  • The PR replaces tests/tools/test_skill_manager_tool.py with +62/-907. Please retain the existing unrelated regression coverage and add focused tests instead.

Suggested changes

  • Port the prompt change to agent/background_review.py.
  • Build on the existing background-review read-before-write guard at tools/skill_manager_tool.py:382-409 (commit 20871c1d941a697723aae24cd8e88afe701793ae) and scope any new autonomous-only enforcement accordingly.

Automated hermes-sweeper review.

Comment thread tests/tools/test_skill_manager_tool.py Outdated
@@ -1,945 +1,100 @@
"""Tests for tools/skill_manager_tool.py — skill creation, editing, and deletion."""
"""Tests for tools/skill_manager_tool.py write-hygiene checks."""

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.

Please retain the existing test module and add focused hygiene cases to it. This rewrite removes 907 lines of unrelated CRUD, validation, path-safety, external-directory, dispatcher, and pinned-skill coverage while adding only 62 lines.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 13, 2026
@gejifeng
gejifeng force-pushed the fix/skill-review-completion-bias branch from 7b980d0 to 6ac681f Compare July 14, 2026 10:36
@gejifeng

Copy link
Copy Markdown
Author

Addressed the latest review and rebased the PR onto current main.

Changes:

  • moved the prompt updates to agent/background_review.py;
  • scoped date-stamp, duplicate-heading, and suspicious-growth checks to autonomous background-review writes only;
  • layered the checks after the existing exact-file read-before-write guard for edit/patch;
  • left foreground user-directed create/edit/patch behavior unchanged;
  • retained the full existing skill-manager test module and added focused background/foreground coverage;
  • updated the existing prompt behavior tests for signal-gated reviews and write hygiene.

Local verification:

  • scripts/run_tests.sh tests/tools/test_skill_manager_tool.py tests/run_agent/test_review_prompt_class_first.py -q (133 passed)
  • background-review regression suite (51 passed)
  • ruff check and git diff --check passed.

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

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades tool/skills Skills system (list, view, manage) type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants