Skip to content

revert: remove hooks-dedupe module, batching guidance block, git-ops idempotency section (evidence refuted) - #328

Merged
Brian Krabach (bkrabach) merged 1 commit into
mainfrom
revert/evidence-hygiene
Aug 28, 2026
Merged

revert: remove hooks-dedupe module, batching guidance block, git-ops idempotency section (evidence refuted)#328
Brian Krabach (bkrabach) merged 1 commit into
mainfrom
revert/evidence-hygiene

Conversation

@bkrabach

Copy link
Copy Markdown
Collaborator

Summary

A payload-level investigation found that three recent changes were built on a measurement artifact: analyses counted the same events 2-4x across duplicate snapshot copies of session files — proven by identical tool_call_id + nanosecond timestamps across the "duplicates." Deduped, the phenomena each change was built to address vanish. This PR reverts all three on that evidence. Not merging — opened for review.


Revert 1 — remove hooks-dedupe module entirely

Added as hooks-tool-dedupe in #323, renamed to hooks-dedupe in #326.

  • Deleted modules/hooks-dedupe/ (whole directory).
  • Removed its hooks: block entry (and the now-empty hooks: key) from behaviors/agents.yaml.
  • Repo-wide grep for hooks-dedupe / hooks_tool_dedupe / tool_dedupe / DedupeHook confirms no dangling functional references remain.

Rationale: the module coalesced "same-batch duplicate reads" that do not exist — deduped count is 0 across 314 sessions / 10,320 reads; measured real value 19k tokens ($0.003) across 5 runs vs. the claimed 16.4% — a ~4,800x overstatement.

Kept: tests/test_hook_module_classification.py (added by #326). It guards a real amplifier-core loader fragility — _guess_from_naming()'s first-match keyword scan can misclassify a hooks-* module as type=tool — which is independent of whether hooks-dedupe itself exists. Removed only the two hooks-dedupe-specific pieces: the hardcoded assert "hooks-dedupe" in HOOK_MODULE_IDS, and the test_hooks_dedupe_passes_real_validation end-to-end test (plus its now-unused ModuleValidationError import). The general, dynamically-parametrized test_hook_module_classifies_as_hook test is untouched and still exercises every remaining hooks-* module.

Revert 2 — remove PR #320's batching guidance block

In modules/tool-delegate/amplifier_module_tool_delegate/__init__.py, removed the "BATCH YOUR DELEGATIONS..." block (through the "...it was independent" bullet) from the delegate tool's rendered description, and restored the original line it replaced: - Launch multiple agents concurrently when tasks are independent, at the top of "Agent usage notes:".

Rationale: measured inert in a clean same-commit 5v5 A/B (treatment waves median 4 vs. control median 3 — the earlier apparent win was a version confound), while costing ~175 tokens on every request.

Revert 3 — remove PR #317's "## Idempotency Discipline" section

Removed the section from agents/git-ops.md (between "Git Safety Protocol" and "Common Git Commands").

Rationale: its evidence ("git-ops re-ran identical clone x8 / ls-remote x7 under context truncation") is refuted — deduped session data shows 8 clones of 8 different repos, once each, with per-repo ls-remote calls. The thrash never happened.


What is deliberately KEPT


Verification

  • Tests: uv run pytest tests/ -q1640 passed, 1 skipped (down from 1642 by exactly the two hooks-dedupe-specific instances removed above: the deleted async test, and one fewer parametrize instance of test_hook_module_classifies_as_hook now that hooks-dedupe no longer exists on disk).
  • Lint/types: python_check clean on all touched files — only pre-existing, unrelated ruff warnings remain elsewhere in tool-delegate/__init__.py (none introduced by this change).
  • Dangling references: repo-wide grep confirms modules/hooks-dedupe/ is gone with no dangling functional references. The 3 remaining textual mentions of hooks-tool-dedupe/hooks-dedupe are historical narrative inside the kept regression test's module docstring and assert message, explaining why that general test exists.

🤖 Generated with Amplifier

…idempotency section (evidence refuted)

A payload-level investigation found that three recent changes were built on
a measurement artifact: analyses counted the same events 2-4x across
duplicate snapshot copies of session files (proven by identical
tool_call_id + nanosecond timestamps across "duplicates"). Deduped, the
phenomena each change was built to address vanish. This reverts all three
on that evidence.

1. Remove hooks-dedupe module entirely (PR #323 as hooks-tool-dedupe,
   renamed in PR #326). Deleted modules/hooks-dedupe/ and its `hooks:`
   entry in behaviors/agents.yaml. Deduped, the "same-batch duplicate
   reads" it coalesces do not exist: 0 across 314 sessions / 10,320 reads.
   Measured real value was ~19k tokens (~$0.003) across 5 runs against a
   claimed 16.4% savings -- a ~4,800x overstatement.

   Kept: tests/test_hook_module_classification.py (added by #326). It
   guards a real amplifier-core loader fragility (name-based module-type
   guessing misclassifying a `hooks-*` module as `tool`) that is unrelated
   to whether hooks-dedupe itself exists. Removed only the hooks-dedupe
   specific references: the hardcoded
   `assert "hooks-dedupe" in HOOK_MODULE_IDS` and the
   `test_hooks_dedupe_passes_real_validation` end-to-end test (and its
   now-unused `ModuleValidationError` import). The general, dynamically
   parametrized `test_hook_module_classifies_as_hook` test is untouched
   and still covers every remaining `hooks-*` module.

2. Remove PR #320's "BATCH YOUR DELEGATIONS" guidance block from the
   delegate tool's description in
   modules/tool-delegate/amplifier_module_tool_delegate/__init__.py,
   restoring the original "- Launch multiple agents concurrently when
   tasks are independent" line it replaced. A clean same-commit 5v5 A/B
   measured the guidance as inert (treatment waves median 4 vs control
   median 3) -- the earlier apparent win was a version confound -- while
   it cost ~175 tokens on every single request.

   PR #320's two context-file edits (context/agents/multi-agent-patterns.md,
   context/agents/delegation-instructions.md) are untouched; only the
   scope named above is in this revert. PR #327's own deletions in this
   same string (the CRITICAL/ALWAYS/NEVER preamble and the "DEFAULT TO
   DELEGATION" line) are also untouched -- they stay removed.

3. Remove PR #317's "## Idempotency Discipline" section from
   agents/git-ops.md. Its evidence -- "git-ops re-ran identical clone x8 /
   ls-remote x7 under context truncation" -- is refuted: deduped session
   data shows 8 clones of 8 *different* repos, each cloned once, with
   per-repo ls-remote calls. The thrash it was written to prevent never
   happened.

Verified: full suite green (uv run pytest tests/ -q: 1640 passed, 1
skipped -- down from 1642 by exactly the two hooks-dedupe-specific test
instances removed in (1), both accounted for). python_check clean on all
touched files (only pre-existing, unrelated ruff warnings remain in
tool-delegate's __init__.py, none introduced by this change). Repo-wide
grep confirms modules/hooks-dedupe/ is gone with no dangling functional
references (three remaining mentions of hooks-tool-dedupe/hooks-dedupe are
historical narrative inside the kept regression test's docstring/assert
message, explaining why that general test exists).

🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)

Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
@bkrabach

Copy link
Copy Markdown
Collaborator Author

Evidence-hygiene revert — documenting comment

A payload-level investigation into the claims underlying the three items this PR removes (hooks-dedupe module, the batching guidance block, and the git-ops idempotency section) found that the underlying measurements were artifacts, not real phenomena:

  • Duplicate session event files on disk: session event files existed 2-4x on disk as snapshot copies. The original analyses counted the same events multiple times as a result — confirmed by identical tool_call_id + nanosecond timestamps across the "duplicate" rows.
  • Deduped re-analysis: once duplicate snapshot copies are removed, the phenomena the original PRs were built on disappear — 0 same-batch duplicate reads across 314 sessions.
  • git-ops "clone x8": re-examined at the payload level, this was 8 different repos, each cloned once — legitimate, non-duplicative work, not context-truncation thrash.
  • Batching guidance block: measured inert in a clean, same-commit 5v5 A/B test — no measurable effect on delegation/batching behavior.

Given all three premises are refuted by the underlying data, this PR reverts the corresponding additions:

  • removes the hooks-dedupe module
  • removes the batching guidance block
  • removes the git-ops idempotency section

Correcting comments are being posted on the originating PRs (#317, #320, #323) linking back to this revert and the evidence above.


Merge process note: this PR is self-authored; self-approval is not possible on this repo, so per the established maintainer admin-merge pattern (documented precedent: ~25 PRs this week), this is being merged via explicit user direction with admin override of the review-required gate. CI is green (6-matrix + license/cla, all passing).

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