docs: add token-friendly AI repository guidance#89
Conversation
008ebc1 to
e3a4362
Compare
tadelv
left a comment
There was a problem hiding this comment.
Review: AI docs routing
The router pattern is a good idea for token efficiency, and AI_GPIO_NOTES.md / AI_BUILD_NOTES.md / AI_REPO_MAP.md are genuinely useful additions (the GPIO pin table and board summary didn't exist before). A few things to address before merge:
Question: "No comments in code" rule
The new AGENTS.md introduces:
- No comments in code; use clear names and small functions.
This rule didn't exist in the original CLAUDE.md, and the existing codebase heavily uses comments — hds.ino has 324 // comment lines plus Chinese comments throughout, websocket.h has 85, menu.h has 120. Many of these comments document non-obvious hardware behavior, thread-safety constraints, and debugging footguns that "clear names" alone can't convey.
What's the rationale for this rule? If it's a token-efficiency measure for AI agents, it should be scoped to "new code written by AI agents" rather than a blanket prohibition. If it's a general coding philosophy, it needs discussion since it contradicts the existing codebase and the maintainer's preferences. I'd like to understand the intent before this lands.
"Prefer many small files" conflicts with unity-build pattern
The new rule says:
- Prefer many small files over a few large files.
- Keep files around 200-400 lines when practical; 800 lines is the ceiling.
The original CLAUDE.md documents that "most logic lives in include/*.h as full implementations" and "treat them as a unity build" — you can't include the same header from two translation units. This is a deliberate architectural choice, not an accident. "Many small files" is reasonable advice in general, but "800 lines is the ceiling" is already violated by hds.ino (2150 lines), menu.h (1460 lines), and pull_ota.h (1368 lines, from #87). Is this an aspirational guideline for new files, or a retroactive mandate? If aspirational, say so explicitly. If retroactive, it needs a plan.
AGENTS.md and CLAUDE.md are identical
The two files are byte-for-byte copies. Having two identical files doubles maintenance — any edit must be made twice or they'll diverge. Consider making CLAUDE.md a one-line pointer: See AGENTS.md. (or vice versa).
Specificity loss in AI_FIRMWARE_NOTES.md
The key actionable rules ARE preserved (gate broadcasts with wsBroadcastHeapOk(), don't call WiFi.setSleep(false), stay in polling mode, don't touch u8g2 from AsyncTCP). But some debugging-critical specifics are lost:
- The
WS_BROADCAST_HEAP_FLOORvalue (~25 KB) and the 15 KB heap watchdog threshold — an agent debugging a heap-exhaustion reboot needs these numbers to interpret serial logs - The specific serial log strings to grep for (
[ws] low heap … skip broadcast,[health] heap=) — the troubleshooting table in the original was a "first place to look" reference; the new version says "Look for WebSocket broadcast OOM and missing heap gates" which is less actionable - The
WS_MAX_QUEUED_MESSAGES=8cap and the 30s ack timeout — these explain WHY connection churn collapses the heap - The ADC
setSamplesInUse()load-step restart behavior — the original explained that placing a calibration weight restarts the ramp, which is the non-obvious part that tripped up PR #71
The original CLAUDE.md was 169 lines because every line was earned through debugging. The new docs are more readable but less precise. Suggestion: keep the router structure but restore the specific numbers, log strings, and thresholds as a "Deep Reference" in each topic file — the router already points there, so token cost is only paid when needed.
AI_STORAGE_NOTES.md references unmerged code
Acknowledged in the PR body — defer commit 4101412 until #88 lands. The AI_REPO_MAP.md also references include/ota_rollback.h, include/pull_ota.h (from #87), and include/grinder_runtime.h (from #82) which don't exist on main yet. Merge #89 last after those PRs land, or split the PR so the router + existing-topic docs go first and the new-file references land with their respective PRs.
AI_BUILD_NOTES.md serial capture
References tools/serial_tap.py — verified this file exists. The reference is valid. Though the original CLAUDE.md had a more complete pyserial snippet for non-PTY shells; serial_tap.py may serve the same purpose but the original snippet was copy-pasteable without knowing the tool exists.
Summary
Dependency
AI_STORAGE_NOTES.md describes include/storage.h from the NVS migration work at ab21642. Merge that storage change first, or defer commit 4101412 until it lands.
Optional
Could be easily enhanced with other category md files like if we have github merge/pr requirements.