Ponytail Audit

Ponytail Audit

Audit a codebase surface for two things: (A) over-engineering — code that should be deleted or collapsed — and (B) missing capabilities — gaps where the system already owns the pieces but doesn't wire them together, or where the documented surface lies about what the code does.

The lens is the user's: Ponytail = xkcd's minimalist engineer. Default to the simplest thing that works. Every "nice to have" is guilty until proven needed. Duplication is a bug. Dead code is debt you ship.

When to use

• User says "audit for over-engineering and missing capabilities".
• User says "review under Ponytail lens" / "Ponytail lens".
• Any request to find bloat, dead code, or gaps in a module/integration/CLI.
• After writing a large feature surface, as a self-review pass before declaring done.

Procedure

0. Look up any unfamiliar lens/tool/term first. If the user references a method, plugin, or lens you don't recognize (e.g. they say "use ponytail" and you don't know it), STOP before doing anything else, search GitHub/web for it, and report back what it is and how it applies. Do NOT guess or proceed blind. (User's explicit rule: "if you don't know what it is, stop and search, then report back.") Only continue once the term is understood — and if it is an installable plugin, review its source for safety (no network calls, no destructive ops) before installing.

1. Map the surface in parallel. Batch read_file / search_files calls — read every file in the target directory in one turn (the runtime executes independent reads concurrently). For a wide surface, enumerate files first with search_files(target='files'), then read the top-level + each sub-area.

2. Build the duplication map. search_files for: function/class names that repeat across files; identical helper bodies; copy-pasted blocks. Greps are your proof — don't eyeball "looks similar".

3. Build the dead-code map. For any class/function/variable you suspect is unused, search_files the whole repo for its name. A symbol is dead only if zero non-definition references exist (tests count as a reference — note them explicitly).

4. Build the claim-accuracy map. Cross-check docstrings/tool-schemas against actual call sites. The most common over-engineering tell is a tool/flag documented but not wired (e.g. a schema field read but never forwarded). Two axes:

   • Code-vs-docs: docstrings/tool-schemas vs actual call sites (above).
   • Imports-vs-manifest: grep every import X / from X import in the audited surface, then check [project.dependencies] and each [project.optional-dependencies] extra in pyproject.toml. A module that imports nacl, argon2, keyring, etc. but whose extra only declares cryptography is a contradiction — either declare the extra or delete the unused branch. Check BOTH the root pyproject.toml AND any sub-package manifest (e.g. integrations/*/pyproject.toml); the core may be zero-dependency while an integration sub-package declares deps — don't conflate the two when judging a "zero-dependency" claim.

5. Look for gaps in (B). Identify subsystems that already exist but are unreachable from a documented entry point (CLI command missing, MCP tool not exposed), or safety/quality passes (PII scan, idempotency, dry-run preview) that exist elsewhere but aren't applied at the highest-risk boundary.

6. Write the report in the structure below. Rank by effort-vs-value.

Verification discipline (no false positives)

• Never call a symbol "dead" without a repo-wide grep proving no other caller.
• Never call a block "duplicate" without confirming it is byte-for-byte (or near-identical) repeated — note divergence points.
• Confirm duplication is fixable (shared base/mixin exists or is cheap to add) before recommending deletion of overrides.
• For "half-wired flags", trace the value from schema → handler → downstream call and show exactly where it's dropped.

Report structure (the format that worked)

A. Over-engineering / delete-or-simplify — each item: File:lines — what it is — Ponytail rung — fix. Use rungs:

• (1) YAGNI — built for a future that hasn't arrived; delete or defer.
• (2) reuse in-codebase — identical logic duplicated N times; lift to one shared base/helper.
• (3) correctness/footgun — misleading or half-wired; fix or remove.
• (5) one line — trivial cleanup (no-op branch, unused import).
• (6) security / minimum that works — never simplify away. Wildcard CORS, auth headers, TLS, constant-time compares, transaction wrappers. These look like "extra code" but removing them is a regression, not a simplification. Flag as KEEP, with reasoning, not as a delete candidate.

B. Missing capabilities (gaps) — each item:

• Idea (one line), Implementation (how, noting zero/low new deps), Plan (numbered steps), Why (what breaks / contradicts today).

C. Top 3 prioritized recommendations — (S)mall / (M)edium effort tags, each a self-contained next action that delivers the most value.

Cross-cutting flag (when it applies). If the package headline makes a sweeping claim (e.g. "Zero-Dependency, Sub-Millisecond", "Fully Private") that the audited subsystem does NOT satisfy, call it out explicitly with proof and a one-line recommendation (scope the claim / guard the heavy import). This is a legitimacy gap, not a style nit — flag it last so it's unmissable.

Common over-engineering signatures (checklist)

Recall / ranking hot-loops (highest-leverage, look FIRST)

These three show up constantly in hybrid vector+FTS scorers and each is a sweeping-claim contradiction ("sub-millisecond" / "O(1)"):

• Per-candidate hot-loop DB query. Inside the scoring loop a SELECT ... WHERE source LIKE '%id%' (leading wildcard → index unusable → full table scan per candidate). Fix: batch all candidate IDs into ONE indexed IN (...) query before the loop, fold into dicts, O(1) lookup inside. See references/recall-rank-hotloop-patterns.md (Pattern 1).

• Multi-tier cache that invalidate()-s on every write. The SQL table / embedding-cosine-scan tiers / TTL machinery become dead weight; the cosine tiers claim "O(1)" but loop all entries per get() = O(n·d). Fix: collapse to ONE in-memory dict. See the same reference (Pattern 3).

• Cache keyed only on the query string while recall() accepts filters (author_id, from_date, memory_type, ...) → cross-scope result bleed (correctness bug, rung 3). Fix: filter-aware key (Pattern 2 in the reference).

• Recomputed loop-invariant inside the per-candidate loop (e.g. a popcount table built every iteration). Hoist to a module-level constant (rung 5).

• A lazy proxy / builder class with 5+ methods that is never instantiated outside its own module → dead scaffold, delete.

• N subclasses each overriding a base method with ~60 lines that reproduce the base plus one identical SQL/loop block → lift the delta into the base, delete overrides.

• A validate() whose only branch is if x: pass → no-op; remove or enforce.

• The same helper function defined identically in 2+ files (e.g. _resolve_default_scope, DB-path resolution) → one shared module.

• A utility method (_content_hash, _slugify) only referenced by a unit test and never in production → either use it or drop it.

• A tool/CLI flag exposed in a schema but silently ignored on one code path → footgun; thread it through or delete from schema.

• A "self-healing" import guard (if not hasattr(logging, "getLogger"): sys.modules.pop("logging"); reimport) defending against an unreachable failure mode → delete (rung 1/5). stdlib modules are always importable.

• Two crypto/transport backends doing the SAME job (e.g. PyNaCl SecretBox duplicating cryptography.fernet.Fernet) → keep the one the extra actually declares; delete the undeclared duplicate (rung 6 — minimum that works).

• A speculative "stub" method that builds a rich data structure for a future LLM/consumer that doesn't exist and is invoked nowhere → dead YAGNI (rung 1).

• A complex algorithm (transitive-closure, fixed-point) implementing a feature the docs label "planned" and that no call site reaches → dead code even though it's impressive; delete (rung 1).

• A dead reassignment / unreachable branch inside an if (re-sets a variable to the value it already holds, imports a module never used) → rung 5.

• A second, divergent extraction/engine module that re-implements an existing subsystem with a different prompt shape / output schema and is reachable only from benchmarks or a self._client field that is never instantiated on the production path. e.g. extraction/client.py (ExtractionClient, flat fact-array prompt) vs core/extraction.py (extract_facts_safe, multi-category MEMORIA prompt) → eval measures a different system than production; collapse to one source of truth (rung 2). Reachability proof: grep the class for instantiation sites outside tests/benchmarks; _extraction_client = None that is only ever set in _benchmarks/ looks unreachable in prod — BUT a non-benchmark test that imports the class directly (e.g. from mnemosyne.extraction.client import ExtractionClient) is proof of life and OVERRIDES the benchmark-only read. When a test constructs the object, the module is live; do NOT delete it. (This session: mnemosyne/extraction/ was flaggable as benchmark-only dead because the only production instantiation is in _benchmarks/; but tests/test_c13b_extraction_diagnostics.py imports ExtractionClient directly and builds it — kept.)

• A full subsystem with zero callers (its def harmonize() / def run() appears only in its own module + docstrings + build/ copies). Grep for the call name repo-wide; if it's never invoked, the whole module (schema creation, LLM loop, helpers) is dead YAGNI (rung 1) — unless a documented entry point is planned, in which case WIRE it now or label it clearly as not-yet-shipped.

• A standalone token_counter.py / cost module with zero importers (grep import token_counter returns nothing) while the same estimate_tokens helper is duplicated in 2+ real modules → delete the standalone module, reuse the shared helper (rung 1 + 2).

• A reverse/decode map with no decoder. REV_CATEGORY / REV_PHRASE style dicts built "for decode" but no decode() function exists anywhere (grep def decode) → the reverse maps are dead; delete them, keep encode().

Common missing-capability patterns (gaps)

• A subsystem (importers, providers) with a registry + import_from_* functions, but the primary CLI only reaches one variant → expose the registry on the CLI (--from <provider>, --list-providers).

• A PII/secret scanner that exists in core/ but isn't run at the ingestion boundary (imports pull raw third-party dumps) → wire it post-transform.

• dry_run that returns only a count → add a preview of first N transformed items so mapping bugs are catchable before commit.

• Import branded "idempotent" but re-inserts blindly for most providers → add a stable content-hash dedup key (reuse the already-written unused hash helper).

• Tool-schema description listing only 2 of 8 supported providers → fix the doc to match the code.

• Adapter↔server contract drift: a client sends since_token but the server reads since, or the client persists a cursor under key X while the engine reads key Y → silent full re-sync every cycle. Fix the key name and align the cursor namespace; add a regression test asserting the cursor advances.

• A import keyring (or any optional lib) inside an adapter with NO entry in any pyproject.toml → fails silently (returns "") when not preinstalled. Either declare the dep or delete the branch.

• Stdlib urllib transport over the internet with a single urlopen and no retry/backoff → wrap in a 2-attempt loop with short exponential backoff (zero new deps). Sync/HTTP utilities should survive one transient drop.

• Deploy healthchecks already probe the server (/sync/status) but there is no lightweight /healthz and no error surfacing in get_status → add a liveness route + an error counter exposed in status (zero deps, big observability win).

• Conflict resolution is last-writer-wins only, silently discarding one side on divergence → add a keep_both mode (sibling memory with a conflict: prefix) with no LLM needed, to preserve data the docs claim to protect.

• Embedding cache asymmetry. embed_query is lru_cached but embed() (documents) is not, so identical content re-embedded on re-index / edit / summary hits the model every time. Mirror the query cache: key on the tuple of prefixed doc texts, share one cache, cap maxsize ~1024. Often the single biggest cost + latency win in an embeddings subsystem, and it's the gap that breaks a "sub-ms / cheap" claim on repeated content.

• No observability on the expensive boundary. Extraction/LLM layers carry full process-global diagnostics (tier counters, success-rate) but the embeddings layer — which makes the actual network/GGUF calls — exposes none (an _API_CALL_COUNT incremented but never read counts as NO observability). Add a tiny mirror of the extraction-diagnostics pattern: API-call count, embed latency, cache hit-rate, local-load-failures. Fold cost logs into the same surface instead of three overlapping, partly-unread cost mechanisms.

• Local→API fallback gap. Embedding transport is chosen once at import by name heuristics; a runtime local-model load/embed failure then yields None and silent recall degradation. If an API key/URL is configured, retry the failed embed via the API (trust-boundary safe — no untrusted input).

• Branding-vs-reality on "zero-dependency / sub-millisecond". When the package headline claims zero-dep / sub-ms, check whether the audited subsystem actually holds: embedding/LLM layers effectively require numpy + optional heavy backends (fastembed, llama-cpp-python, ctransformers, huggingface-hub) and are network/GGUF-bound (ms–seconds, never sub-ms). The claim holds only for the FTS5 + lightweight-fallback recall path. Either guard the heavy imports behind a clean optional path or scope the claim in docs so the layer isn't misrepresented. Don't let the marketing line mask real cost.

• Veracity / trust-ranking silent bugs. In a "Bayesian veracity" layer, check the weight table ordering (unknown must be the LOWEST tier, not above inferred/tool/imported), and reconcile docstrings vs the actual formula (1 - 0.7^n claimed but an incremental old + (1-old)*w*0.3 implemented). Also flag auto-resolve paths that ignore veracity/recency and contradict a richer conflict model elsewhere in the same module.

• Security gaps that can masquerade as over-engineering but are MUST-FIX (rung 6 — never list these in section A as delete candidates): a Ponytail sweep over an agent/memory system must actively hunt for these, because they are the highest-severity findings and are easy to misclassify as "extra code":

  • Stored-memory → prompt injection. Any path that renders stored content into agent prompt text (persona renderers, model-card builders, recall/export) MUST strip injection markers AND gate on a trust_tier/trust field. If the rendered output is persisted to disk (e.g. persona.md), re-sanitize from source on regenerate — otherwise a later forget() leaves the injected instruction live. This is the only class that yields direct agent control from data the system is built to store. Fix: shared strip_injection_markers() + redact_secrets() (reuse existing secret regexes), called at every render/export boundary.
  • Right-to-be-forgotten not honored. forget()/end() that only stamp valid_until (soft delete) while export_all()/history() still return the row, and no physical purge() exists. Fix: add purge() (physical DELETE, owner-scoped), default export_all(include_history=False), document soft vs hard, wire a --hard forget tool.
  • Non-atomic durable writes. write_bytes/direct f.write with no temp+os.replace+fsync → corrupt payload on crash; an installer regex rewrite of a YAML config that silently drops sibling keys; rmtree of a real user directory with no force guard. Fix: temp+rename+fsync for blobs/config; real YAML parse/emit instead of regex; guard real-dir removal behind force.
  • Plaintext PII-at-rest in DR. create_backup() gzip-dumps the whole DB unencrypted; restore_backup() trusts any consistent file, ignoring a computed backup_checksum. Fix: encrypt backups (reuse existing sync-key / secrets+hashlib), verify checksum before restore.
  • Plugin/extension dir auto-executes arbitrary code. A directory globbed at startup and exec_module'd with no allowlist/signature → local-write gives full RCE in the process that holds all PII. Fix: gate behind an explicit allowlist in config; log each loaded plugin.
  • Unvalidated store inputs. owner_id/keys accepted unbounded; numeric confidence accepts NaN/-inf/out-of-range. Fix: length caps on key/text columns; clamp confidence to finite float ∈ [0,1]; document that callers authorize owner_id/scope. Surface these as CRITICAL/HIGH must-fix items in section B, never in section A.

Localhost-bound API plugins (agent / bridge / sidecar setups)

A very common minimal-setup pattern: a tiny HTTP API (aiohttp / flask / fastapi / Express / python -m http.server) runs on 127.0.0.1 to feed a local client (a MagicMirror module, an Electron renderer, a TUI, a dev dashboard). These look "harmless — it's localhost" but the Ponytail security sweep MUST treat them as internet-facing the moment the deployment story adds a second machine (Raspberry Pi over LAN, container-to-host, remote dev). Audit checklist:

• No auth + free resource params = full read. If session_id / id is a query param with no owner check, any caller enumerates every record.
• Access-Control-Allow-Origin: * is dead weight for a Node/Python client (those clients don't enforce CORS) and is a localhost drive-by exfil vector today (a malicious page can fetch('http://127.0.0.1:PORT')). Delete the header; if a browser truly needs it later, set an explicit origin.
• Raw DB transcripts / PII served verbatim (LCM / SQLite / kanban dumps) = secret-disclosure path. Add a one-line redaction regex on sk- / AKIA / Bearer / ghp_ shapes BEFORE serializing. Heuristic, not a sanitizer — label it with a ponytail: comment.
• Hardcoded profile/path (profiles/senna/) is a correctness/trust smell; derive from an env var (HERMES_PROFILE).
• LAN-exposure escalation (CRITICAL in the planned deploy): host hardcoded 127.0.0.1 with no knob. The moment it's bound 0.0.0.0 to reach a Pi / container, zero-auth exposes everything to the LAN. Minimal fix: make host configurable AND refuse non-loopback bind unless an API token is set — couple "exposed" to "authenticated" so it can't be opened by accident.
• Minimal hardening that stays lazy: one shared X-Api-Token check (fail-closed, hmac.compare_digest), drop the CORS wildcard, redact transcripts, configurable host gated on token. ~10 lines/plugin. No SQLi if queries are parameterized — verify that first; it's the cheap win. See references/localhost-api-plugin-security.md for a worked example.

Phase 2 — Delegate the fixes (audit → improve)

The user's usual next step after an audit is "delegate the team to complete the fixes." Run it as a multi-wave delegation, NOT one batch, because subagents editing the same file concurrently clobber each other.

Wave discipline:

• Group fixes so each wave's subagents own disjoint files — split by subsystem (security, embeddings, importers, sync, recall). Two specialists must NOT both touch cli.py/mcp_tools.py in the same wave.
• If a file is legitimately needed by two specialists (e.g. security's cli.py B2 + importer's cli.py A4), let ONE own it and note the intersection; reconcile that file yourself when the batch lands (re-read, merge both diffs, retest) rather than letting two agents race.
• Each subagent brief must include: the audit findings it owns (cite file:line), exact files to edit, the Ponytail safety carve-outs (never remove validation/security), verification commands, and a strict git guard: git branch --show-current must be the feature branch; NEVER git add -A; git add ONLY owned files; NEVER git push; focused commit message; retry on lock up to 3×.
• Dispatch with delegate_task in background; results re-enter as one batched message. Do not poll.
• After each wave: git status + git log --oneline -n, run the suite, reconcile any shared-file intersections, then launch the next wave.
• Honesty gate: if a fix can't be made safely (live caller found, tests break unfixably), the subagent returns it NOT-DONE-with-why; report that to the user, never fabricate success.

See references/audit-to-fix-wave-playbook.md for the exact subagent brief template and git-commit guard.

Pitfalls

• Don't capture environment-dependent failures (missing binaries, uninstalled packages, command not found) as audit findings — those are setup, not code debt.
• Wildcard CORS (Access-Control-Allow-Origin: *), auth/security headers, TLS setup, and transaction wrappers are NOT over-engineering even though they add lines. Never recommend deleting them as "simplification" — classify as KEEP (rung 6) and explain why. The audit must not trade security for brevity.
• When judging an extras/packaging claim, separate the root package manifest from sub-package manifests; an integration sub-package legitimately declares deps while the core stays zero-dep. The contradiction to flag is code importing an undeclared lib, not the existence of deps in a sub-package.
• Don't mirror upstream docs into the skill; capture condensed, reusable patterns.
• Classify every dead-code grep hit as prod | test | benchmark before concluding "dead." The repo-wide grep is mandatory, but the reading of its hits needs triage. A symbol is genuinely dead only if EVERY reference is (a) its own definition, or (b) a _benchmarks/ call site with no production path. A non-benchmark test that imports/constructs it directly is proof of life — even when no production module constructs it (production may reach it lazily via getattr / a = None field set elsewhere, or the test IS the production smoke test). Concrete case this session: grep showed ExtractionClient instantiated only in _benchmarks/, which the audit's own rule would call "unreachable in prod" — yet tests/test_c13b_extraction_diagnostics.py does from mnemosyne.extraction.client import ExtractionClient and builds it. That makes it live. Rule: when a test imports the class confirms it constructs the object; if yes, KEEP. Report the triage (prod/test/benchmark counts) in the finding.
• A public facade / core/__init__.py export is proof of life too. A symbol that looks speculative because its only direct importers are tests can still be live via an indirection you'd skip: a property on the main Memory/Client class (e.g. memory.compressor lazily builds MemoryCompressor), plus an export in core/__init__.py. Grep BOTH the lazy self.<x> = facade AND the package __init__, not just import SymbolName. Case this session: MemoryCompressor had no production import but WAS live via the memory.py compressor property + core/__init__.py export + test_patterns.py → kept. When in doubt about a class with a facade, don't delete; collapse-tier or defer instead.
• Don't delete a symbol mid-fix without re-grepping its new import sites. If your own edit moved a module (e.g. git mv chat_normalize.py _benchmarks/ to fix a "core vs benchmark" leak), update the importer's from line too — the file still exists, just relocated; the dead-code verdict was "move to _benchmarks/", not "delete".
• Don't over-prioritize cosmetic cleanups over the one duplicated base method that, if fixed, deletes 300 lines. Lead with the biggest leverage.
• The "Ponytail lens" is the user's reusable methodology, not a one-off task — keep it as the framing, not as a dated artifact.