Skip to content

docs(agents): 5 new avoided patterns + planner self-audit step#8377

Merged
HydraOps-T-rav merged 2 commits intomainfrom
plan-quality-patterns
Apr 21, 2026
Merged

docs(agents): 5 new avoided patterns + planner self-audit step#8377
HydraOps-T-rav merged 2 commits intomainfrom
plan-quality-patterns

Conversation

@HydraOps-T-rav
Copy link
Copy Markdown
Collaborator

Follow-up to #8374 / #8375. Translates the session-level retrospective insights into the factory's persistent workflow.

Why

PR #8374 went through 8 task commits + 8 fix commits (review→fix→re-review). Most fix commits addressed the same class of issues that `docs/agents/avoided-patterns.md` already documents — private symbols crossing modules, missing conftest reuse, unsafe logger formatting, etc. Writing these into the canonical pattern doc means the planner phase (and any sensor/audit agent reading the doc) catches them at plan-writing time instead of in code-review-fix loops.

Changes

  1. `docs/agents/avoided-patterns.md` — 5 new sections in the same Wrong / Right / Why / How-to-check format as existing entries:

    • Underscore-prefixed names imported across modules
    • Writing a new test helper without checking conftest
    • `logger.error(value)` without a format string
    • Hardcoded path lists that duplicate filesystem state
    • `name` for unused loop variables (prefer bare ``)
  2. `src/planner.py` — add Step 8 to the planner's Planning Steps section, instructing it to self-audit the plan against `avoided-patterns.md` BEFORE emitting. Catching these in a plan is orders of magnitude cheaper than catching them in agent code review.

  3. `tests/test_planner.py` — bump `test_build_prompt_truncates_long_body` threshold from `< 10_000` to `< 11_000`. The new Step 8 grew the planner prompt baseline by ~650 chars; the truncation semantic ("…(truncated)" marker + body dramatically smaller than original 20k) is unchanged.

Test plan

  • `make quality` green locally (after test-threshold fix): 11,038 passed.
  • Planner-prompt-truncation test still asserts truncation behavior (just with the new baseline accounted for).

Follow-ups (not in this PR)

Per `avoided-patterns.md`'s own "Adding a new avoided pattern" guidance, consider:

  • Adding rules to `src/sensor_rules.py` so the sensor enricher surfaces these hints automatically on matching failures.
  • Updating `.claude/commands/hf.audit-code.md` Agent 5 (convention drift) to check for the new patterns on sweeps.

Both are separate concerns — doing them requires understanding those subsystems in depth. Filed as mental follow-ups; happy to do either as a separate PR if desired.

🤖 Generated with Claude Code

T-rav-Hydra-Ops added 2 commits April 21, 2026 15:05
Retrospective of PR #8374/#8375 surfaced five recurring anti-patterns that the
plan review caught repeatedly. Document each in the canonical
`docs/agents/avoided-patterns.md` (the sensor enricher and audit agents already
read this file) and add a Planning-Steps self-audit reference so the planner
phase catches them BEFORE emitting a plan.

New patterns:
- Underscore-prefixed names imported across modules
- Writing a new test helper without checking conftest
- logger.error(value) without a format string
- Hardcoded path lists that duplicate filesystem state
- _name for unused loop variables (prefer bare _)

Catching these at plan-writing time prevents the agent-review-fix loop cost
observed in PR #8374 (8 fix commits beyond the initial 8 task commits).
…teps block

Adding the new step 8 (self-audit against avoided-patterns.md) grew the
planner prompt baseline by ~650 chars. The truncation test's `< 10_000`
sanity bound was tight; bump to 11_000. Semantic is unchanged —
"…(truncated)" marker assertion still verifies body truncation works
and the prompt remains well under the original 20k body size.
@HydraOps-T-rav HydraOps-T-rav merged commit b5b8a0a into main Apr 21, 2026
20 checks passed
@HydraOps-T-rav HydraOps-T-rav deleted the plan-quality-patterns branch April 21, 2026 21:31
HydraOps-T-rav added a commit that referenced this pull request Apr 21, 2026
…8378)

* feat(audit): propagate 5 new avoided patterns to sensor rules + audit Agent 5

Follow-up to #8377. The 5 new avoided-patterns entries need matching hooks
in the two automated surfaces that consume the doc.

Sensor enricher (src/sensor_rules.py) — 3 new Rule entries:
- private-symbol-cross-module: pyright "_name is not accessed" trigger
- logger-format-typeerror: TypeError on malformed format strings
- dockerfile-python-constant-drift: Dockerfile* file-changed trigger

(Skipped sensor rules for the two patterns that don't map cleanly to
error/file triggers: "test helper duplicating conftest" and "hardcoded
path list mirroring fs state" rely on diff analysis the sensor runtime
doesn't do. Those remain covered by avoided-patterns.md + Agent 5 sweep.)

Audit Agent 5 (.claude/commands/hf.audit-code.md) Phase 2 — 5 new
detection bullets with concrete rg heuristics:
- Underscore-prefixed names imported across modules
- Test helpers duplicating conftest fixtures
- logger.error(value) without format string
- Hardcoded path lists mirroring filesystem state
- _name for unused loop variables

* fix(audit): add phase_skills to audit_prompts fake config

PR #8374 added `phase_skills: dict[str, list[str]]` to HydraFlowConfig and
each factory runner now reads `self._config.phase_skills` during prompt
construction. PR #8376's prompt-audit harness uses a fake config object
that raises AttributeError on unknown attributes; `phase_skills` wasn't
added to it, so every audit_prompts fixture that invokes a runner's
prompt builder failed with `AttributeError: phase_skills` under CI's
full test run (the failures didn't surface in `make quality` because
that target deselects some tests).

Mirror the existing `required_plugins: list[str] = []` convention — both
are allowlist fields that default to empty in the audit harness.

---------

Co-authored-by: T-rav-Hydra-Ops <t@koderex.dev>
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