Skip to content

[docs] - Record the 2026-10-13 ponytail audit of dead code and duplication - #1638

Merged
cgfixit merged 6 commits into
mainfrom
claude/ponytail-audit-2026-10-13
Oct 13, 2026
Merged

cgfixit merged 6 commits into
mainfrom
claude/ponytail-audit-2026-10-13

Conversation

@cgfixit

@cgfixit cgfixit commented Oct 13, 2026 •

Copy link
Copy Markdown
Owner

Branch naming (required for agent-opened PRs)

claude/ponytail-audit-2026-10-13 — Claude Code prefix. (The session was assigned cc/beautiful-curie-fozcxd; the tracked pre-commit hook refused that prefix and the owner approved the rename rather than a hook override.)

Title

[docs] - Record the 2026-10-13 ponytail audit of dead code and duplication

Proposed changes

Adds docs/audits/2026-10-13-ponytail-audit.md, a read-only "ponytail" (lazy-senior-dev: YAGNI, stdlib-first, minimal abstraction, no dead code, no speculative generality) audit of main at 6519135. Nothing else in the tree changes. (Commit 8731696 is the owner's merge of main into this branch; 8897c91, 40e04da and 6cfa881 correct rows after three Codex review rounds, see Further comments.)

Three independent static passes over the 227 non-test Python files, each built on stdlib ast plus word-boundary grep over all 1,049 tracked files, and every row confirmed by reading its consumer:

  1. Dead code — unreferenced defs/classes/methods/constants after filtering FastAPI handlers, LangGraph nodes, name-dispatched hooks and test seams.
  2. Abstraction — one-use helpers, 8-line sliding-window duplicate hashes, pure forwarders.
  3. Speculative generality — unread config.yaml keys, undocumented/unread CYCLAW_* env vars, ruff --select ARG unused parameters, low-import-count dependencies.

Headline findings (details, line refs and risk per row in the report):

  • utils/numbat_emitter.py:329 _source_agent reads config and then returns DEFAULT_SOURCE_AGENT on every path (dead logic, harmless).
  • utils/authn_cli.py:62 _resolve_config_path has drifted from the other five copies (no .resolve()); the fix must resolve inside load_config's try to keep the EXIT_ENV contract.
  • telegram/runner.py inbound gate duplicated byte-for-byte (modulo blank lines) at :182 and :308.
  • patching.py _release_artifact_lock is byte-identical to registry.py _release_registry_lock.
  • ~70 lines of never-called mock scaffolding in both Sandbox skill copies; two unused parameters worth trimming (ruff ARG001). The two unread cfg kwargs on graph.py nodes are listed as a deliberate keep: 13 positional callers.
  • Four config.yaml keys no code reads (app.name, app.env, models.embeddings.provider, policy.fallback.enabled); app.debug was removed by [fix] - Remove the inert app.debug key and correct a stale agentic telemetry comment #1636 (now merged) and is credited, not re-proposed.
  • Three env vars read in code but documented nowhere (CYCLAW_PYTHON, CYCLAW_LOCAL_LLM_API_KEY, CYCLAW_LOCAL_LLM_FALLBACK_API_KEY).
  • No runtime dependency should be dropped; rank-bm25 is the only plausibly stdlib-replaceable one and the report says why not.
  • Of the five candidates deferred by the 2026-09-11 audit, three remain and two are gone.

Potential: about 320 source lines in Tiers 1-2, zero dependencies; plus ~1,750 lines of archived scripts under docs/ and ~420 lines of test-only production code that are the owner's call. Nothing was applied.

Invariant / Governance Impact: none. Docs-only; no code, config, edge, router, soul path or import changes. The report's Invariants section argues each candidate against I1-I6 for whoever picks them up, and states that no graph.py change is proposed.

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Security (auth, privacy, injection, egress, redaction, or secrets)
  • Invariant / Governance refinement (use this for changes that strengthen or evolve the 6 invariants, I6 isolation, or harness phases)
  • Dependencies (added, removed, or bumped)
  • CI / infrastructure (workflows, hooks, templates, or checkers)
  • Refactor or tests (behavior-preserving, no new capability)
  • Documentation update
  • Other (none of the boxes above fit)

Scope note: Docs + audits.

Benefits / why

  • Gives the owner a verified, tiered cut list instead of a vague "simplify" instinct: each row names the file:line, call-site count, estimated lines, and the specific risk (tested-by-name, security contract, owner-retained).
  • Catches two correctness-adjacent items that a pure line-count audit would miss: the always-default _source_agent and the drifted authn_cli resolver.
  • Re-closes the loop on the 2026-09-11 deferred list so the same candidates are not re-discovered a third time.

Risks to monitor

  • A "dead" claim that is wrong. Mitigation: name-count method is a floor, every row was read, headline rows re-verified by a second pass, and three Codex review rounds caught eight imprecise sentences that are now corrected; UNSURE rows are marked. Anyone applying a cut should still grep the name once more.
  • Someone applying the config.yaml rows without re-checking against current main (app.debug is already gone since [fix] - Remove the inert app.debug key and correct a stale agentic telemetry comment #1636). The report says to trial-merge first.
  • Someone applying the label trim on one side only, or the authn_cli resolve outside the try: the report now spells out both.

Checklist

  • I have read docs/THREAT_MODEL.md, INVARIANTS.md and SECURITY.md (and any relevant Phase docs)
  • This change preserves all 6 security invariants and I6 module isolation (docs-only)
  • Full sandbox validation has been run — not run; docs-only, nothing executable changed
  • No new external network dependencies or mandatory online LLM assumptions were introduced
  • For any agentic/fsconnect change: n/a
  • Relevant architecture docs or threat model notes updated — n/a, no behavior change
  • Commit messages follow the title prefix convention above
  • For large or complex changes: n/a

Further comments

Checks run directly in the session: ruff check --select F401,F811,F841,B007,ARG --exclude .claude,tests,tools/lora_finetune/tests . (ruff 0.16.8; 57 hits = 52 ARG* in non-test code + 5 vendored F401, all triaged in the report; the unscoped walk reports 1,026, almost all ARG* inside tests/, not triaged), python3 .claude/skills/doc-sync/doc_sync.py (0 drift, re-run after each correction), the three stdlib ast/grep passes, a Python 3.12 reproduction of Path.resolve() raising on a symlink loop, and hand re-verification of every Tier 1 claim. Not run locally: the test suite, --cov, ci_rag_smoke.py, any server; CI ran them green on ddfd29a, 8731696 and 8897c91 and re-runs on each later head.

Codex round 1 on 8731696 (four P2, corrected in 8897c91): the graph.py cfg parameters have 13 positional callers (row moved to a keep); the OpenTweet preamble set is two commands not four (cmd_post keeps its EXIT_ENV contract); tests/judge_eval.py:401 sets models.local_llm.fallback.enabled and is dropped as evidence; ruff check . skips docs/memories/zOld/ via docs/memories/.gitignore while the archived script shows 7 hits when named explicitly.

Codex round 2 on 8897c91 (three P2, corrected in 40e04da): the Invariants paragraph still described the withdrawn graph edit (now says no graph change is proposed); the label row omitted the bridge's internal label=label call (now names both edits); the 57-hit ruff figure lacked its exclusions (now documented with the per-rule split and the unscoped 1,026).

Codex round 3 on 40e04da (one P2, corrected in 6cfa881): the authn_cli resolver fix must happen inside load_config's try (or translate OSError/RuntimeError), since Path.resolve() raises on a symlink loop and would otherwise bypass the AuthConfigError → EXIT_ENV contract; the sibling resolvers share the unguarded shape and are flagged. All eight threads answered and resolved.

Suggested merge order of open PRs

Trial merges verified (git merge --no-commit --no-ff + abort) against main @ 663a8bb on 2026-10-13: all four open PR branches merge clean. CI state of the other three was not checked from this session.

#1637 · Safe ✅
Impact: Telemetry egress inventory classification (docs/skill files).
Action: Merge any time. Trial merge clean vs main.

#1638 (this PR) · Safe ✅
Impact: One new file under docs/audits/; already contains main @ 663a8bb.
Action: Merge any time; auto-merge is enabled. Trial merge clean.

#1640 · Safe ✅
Impact: Dated dependency-sweep report under docs/audits/; no overlap with this PR's file.
Action: Merge any time. Trial merge clean vs main.

#1641 · Blocked ⛔ (still a draft)
Impact: bwrap AppArmor policy proof on Ubuntu 26.04 (infra).
Action: Author marks ready first. Trial merge clean vs main as of this check.

ELI5

Someone asked for a "ponytail" pass: the lazy-senior-dev check for code that nobody calls, helpers that exist for one caller, copy-pasted blocks, config knobs nothing reads, and libraries we barely use. This PR adds the written-up result and changes nothing else; the third commit is the owner pulling the latest main into the branch, and the last three fix sentences the Codex reviewer caught across three rounds. The report found about 320 lines that could go with no behavior change (the biggest: a duplicated database opener, a duplicated Telegram inbound check, a lock-release function copied verbatim, and dead mock scaffolding in the sandbox skill), one function that reads a setting and then ignores it (_source_agent in the Numbat emitter), one config-path helper that drifted from its five siblings (and how to fix it without breaking the CLI's error exit), four config keys no code reads, and three environment variables the code honors but the docs never mention. It also says what to leave alone and why: no dependency is worth dropping, the guardrail scaffolds are kept on purpose, the look-alike security handlers are different contracts, and two unused graph-node parameters stay because thirteen callers pass them. Nothing was applied; the owner picks. If a cut from the list ever breaks something, the signal is a failing test that monkeypatches the removed name, which is why each row says whether tests reference it. Not run here: the test suite and coverage gate; CI runs them on each head.

🤖 Generated with Claude Code

https://claude.ai/code/session_014SrcnCP6CA6YjCwnWwa8vC


Generated by Claude Code

Last updated: 2026-10-13 02:13 ET

…ation

Read-only YAGNI / stdlib-first / minimal-abstraction audit against main at
6519135. Three static passes (dead symbols, one-use helpers and 8-line
duplicate windows, unread config keys / env vars / parameters / low-use
dependencies), each row confirmed by reading its consumer. Tiers the
candidates (about 330 source lines in Tiers 1-2, zero dependencies) and
re-checks the five 2026-09-11 deferred items. No code changed.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014SrcnCP6CA6YjCwnWwa8vC
@github-actions

github-actions Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

PR is a complete fill of .github/PULL_REQUEST_TEMPLATE.md (every section, ticked boxes, merge order, ELI5 last, last-updated stamp, title, branch).

@cgfixit
cgfixit marked this pull request as ready for review October 13, 2026 05:37
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-13T06:11:58.020364Z 40e04da New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@cgfixit
cgfixit enabled auto-merge October 13, 2026 05:37
@cgfixit
cgfixit disabled auto-merge October 13, 2026 05:38
@cgfixit
cgfixit enabled auto-merge October 13, 2026 05:38

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 87316966cd

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread docs/audits/2026-10-13-ponytail-audit.md Outdated
Comment thread docs/audits/2026-10-13-ponytail-audit.md Outdated
Comment thread docs/audits/2026-10-13-ponytail-audit.md Outdated
Comment thread docs/audits/2026-10-13-ponytail-audit.md Outdated
The two unread cfg parameters on graph.py nodes have 13 positional callers
(both Sandbox verifiers, ci_rag_smoke.py, test_rerank.py, test_graph.py), so
the row moves from a Tier 1 cut to a keep. The OpenTweet preamble set is two
commands, not four: cmd_post refuses with EXIT_ENV and must keep that
contract. tests/judge_eval.py:401 sets models.local_llm.fallback.enabled, a
live switch, not policy.fallback.enabled, and is dropped as evidence. The
lint-posture line now states that the repo-wide ruff walk skips
docs/memories/zOld/ while the archived script shows 7 hits when named
explicitly. Totals adjusted (about 320 lines in Tiers 1-2).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014SrcnCP6CA6YjCwnWwa8vC

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8897c917b4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread docs/audits/2026-10-13-ponytail-audit.md Outdated
Comment thread docs/audits/2026-10-13-ponytail-audit.md Outdated
Comment thread docs/audits/2026-10-13-ponytail-audit.md Outdated
The Invariants section still described removing the graph partial keyword
after that row became a keep; it now states no graph.py change is proposed.
The label row now names both edits that travel together (the broker
parameter and the label=label keyword in utils/guardrail_bridge.py:105),
since guardrails ship enabled and removing one side alone raises TypeError.
The Verification section now gives the exact ruff invocation behind the 57
figure (--exclude .claude,tests,tools/lora_finetune/tests; 52 ARG + 5
vendored F401, with the per-rule split) and states that the unscoped walk
reports 1,026, almost all ARG inside tests/, which the audit did not triage.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014SrcnCP6CA6YjCwnWwa8vC

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 40e04da8bc

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread docs/audits/2026-10-13-ponytail-audit.md Outdated
load_config resolves the config path before its try block, and on 3.12
Path.resolve() raises RuntimeError on a symlink loop (and can raise
OSError), so adding .resolve() as a bare one-liner would turn a typed
AuthConfigError / EXIT_ENV failure into a traceback. The row now says to
resolve inside the try or translate the failures, and notes the sibling
resolvers in metrics.py and retrieval/indexer.py share the unguarded shape.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014SrcnCP6CA6YjCwnWwa8vC
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