Skip to content

Commit eea8e87

Browse files
0xAHAclaude
andcommitted
docs: add verification rules to claude.md from this week's failures
Twelve rules, each written from a bug that shipped rather than from principle. The ones that cost the most: - Check the protocol docs before inferring. Register meanings were argued across issue threads for a week, twice wrongly, while the answers sat in Protocols/ and docs/developer/protocol-v139.md. SOH is 1096, not 31218; 1021 is grid import and 1037 is house load. - Input and holding registers overlap at the same addresses. Three separate errors, including reading input 43 and concluding a device reports no DTC when the DTC lives in holding 43 -- made while warning another person about the same trap. - Read OK does not mean works. A register answering 0 reports as Read OK, which kept two dead write registers alive through three contradicting field reports. - Verify the whole chain. Three releases needed follow-ups this week and all three had the same shape: the half being thought about was correct and the other half was not. - hasattr() gates are no-ops against dataclass fields. 31 of them exist. - Never write file content through PowerShell. Also adds release cadence and the pull procedure (a draft is orphaned if you delete its tag and must be removed through the API), and PowerShell notes for the bash idioms that fail here. Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 7a73a55 commit eea8e87

1 file changed

Lines changed: 194 additions & 2 deletions

File tree

claude.md

Lines changed: 194 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,134 @@ This document provides comprehensive guidelines for AI assistants (and developer
44

55
---
66

7+
## 🛑 RULES THAT PREVENT REAL BUGS
8+
9+
Every rule below exists because it was broken and something shipped wrong. The examples are
10+
real, not hypothetical. Read this before the sensor checklist.
11+
12+
### 1. Check the protocol documents before inferring anything
13+
14+
We have ~2.5 MB of Growatt protocol documentation in `Protocols/` and a 105 KB extracted
15+
reference in `docs/developer/protocol-v139.md`. **Search it first.** Grep the register
16+
number. It takes seconds.
17+
18+
Register meanings were argued across GitHub threads for a week, twice wrongly, while the
19+
answers were already checked in:
20+
21+
- Battery SOH was reported to a user as register 31218 (VPP range) and possibly
22+
unobtainable. It is **1096**, documented as `BMS_SOH` under "BMS information 1082-1124".
23+
- Registers 1021/1037 were disputed for days. The doc states plainly: `1021 PactouserTotal`
24+
(grid import), `1037 PLocalLoad total` (house load).
25+
26+
A profile's `desc` string, a code comment, and a model's marketing name are **not
27+
evidence**. The protocol document and a field scan are.
28+
29+
### 2. Input and holding registers overlap — always state the function code
30+
31+
The same address means different things in each space. This has caused three separate
32+
errors:
33+
34+
| Address | Holding (FC03) | Input (FC04) |
35+
|---|---|---|
36+
| 43 | DTC / device type code | `Iac2` phase 2 current |
37+
| 1083-1088 | Grid First time periods | BMS status / SOC / voltage / current / temp |
38+
| 1100-1108 | Battery First time slots | BMS gauge and version data |
39+
40+
Reading input 43 and concluding a device reports no DTC — when the DTC lives in holding 43
41+
— is a mistake that has been made *while warning someone else about the same trap*. When
42+
you cite a register, say which space it is in.
43+
44+
### 3. "Read OK" and "responds" do not mean "works"
45+
46+
A register answering with `0` reports as **Read OK**. That is evidence the address
47+
responds, never that the value is meaningful or that writes take effect.
48+
49+
Registers 1071/1091 survived three contradicting field reports because an old scan showed
50+
"all Read OK". They accept writes and silently ignore them.
51+
52+
### 4. Absence of evidence needs the evidence to have been possible
53+
54+
Before concluding a register is empty or absent, confirm the scan actually covered it.
55+
A v0.7.7 scan was used to prove a device reports no DTC — that scanner version never read
56+
the legacy holding range at all. A missing range in a scan is not a missing value.
57+
58+
### 5. Verify the whole chain, not the half you are thinking about
59+
60+
Three releases needed follow-ups this week, all the same shape:
61+
62+
- v1.2.0: the read path consumed the block-size option correctly, and the form could never
63+
save it. Verified the consumer, not the producer.
64+
- v1.3.5: fixed the option format, updated one of two fetch paths. The other raised
65+
`ValueError` on every poll.
66+
- v1.4.0: removed a register so its sensor would disappear. The sensor platform recreated
67+
it from the profile's sensor set and it reported `0.0`.
68+
69+
**When you change a stored format, grep every consumer. When you remove data, check what
70+
recreates it.**
71+
72+
### 6. `hasattr()` gates only work on dynamic attributes
73+
74+
`condition: lambda data: hasattr(data, 'x')` reads as "only if the profile provides x". It
75+
is a no-op when `x` is a `GrowattData` dataclass field, because the field always exists
76+
with a default — the sensor is created regardless and publishes the default, typically a
77+
plausible-looking `0`.
78+
79+
It works only for attributes assigned dynamically via `setattr()` (the BMS block, for
80+
example). 31 conditions in `sensor.py` are decorative; `tests/test_sensor_conditions.py`
81+
enumerates them and fails if a new one appears. To exclude a sensor for real, remove it
82+
from the profile's **sensor set** — that is the only hard filter.
83+
84+
### 7. Never write file content through PowerShell
85+
86+
Use the **Edit** and **Write** tools. `Set-Content -Encoding utf8` writes a BOM in PS 5.1,
87+
which broke `manifest.json` and would have stopped the integration loading. Em-dashes have
88+
been mangled into `â€"` in three separate files this way.
89+
90+
Prefer plain ASCII in commit messages and shell-adjacent content — hyphens rather than
91+
em-dashes, straight quotes rather than curly. Non-ASCII in source files is fine *via the
92+
editing tools*, never via a shell redirect.
93+
94+
Also: `Get-Content -Raw` misreads UTF-8 and will show you mojibake that is not in the file.
95+
Verify encoding claims with `Grep` or Python, not PowerShell.
96+
97+
### 8. Duplicate dict keys are invisible after import
98+
99+
Python silently keeps the last one. `tl_xh.py` defined register `3136` twice; the
100+
temperature mapping never existed at runtime — no sensor, no error, nothing to notice.
101+
Loaded `REGISTER_MAPS` cannot show this. `tests/test_profile_integrity.py` parses the
102+
profile sources with `ast` to catch it.
103+
104+
### 9. Prove a new test fails without the fix
105+
106+
Disable the fix, run the test, confirm it goes red, restore. Assertions written alongside a
107+
fix inherit its blind spots — a block-size test asserted `resolve_block_size(stored) == 25`,
108+
which called the helper on its own output and passed throughout the regression it was
109+
meant to catch.
110+
111+
### 10. Read every comment on an issue before assessing it
112+
113+
Fetching the last 3 of 16 comments has twice produced an "assessment" that missed the
114+
decisive measurement. The comment count is in the same API response — check it. A
115+
maintainer asking "are there new comments?" should never be how a five-comment exchange
116+
gets discovered.
117+
118+
### 11. Two readings agreeing proves nothing; diverging proves independence
119+
120+
Two registers matching at one moment can be coincidence. Two registers diverging at any
121+
moment is proof they are separate sources. Only the second direction is conclusive.
122+
123+
Register 3176 was reported as a duplicate of register 93 after both read `545` in one scan.
124+
A paired reading at a different operating point refuted it. **Always ask for a second
125+
sample at a different operating point before remapping a register.**
126+
127+
### 12. Mark a mapping CONFIRMED only with a citable device report
128+
129+
`DTC_REGISTRY` in `auto_detection.py` records `CONFIRMED` or `ASSUMED` plus evidence.
130+
CONFIRMED means a real device on that DTC was seen running that profile, traceable to an
131+
issue number or scan. Anything else is ASSUMED and says so in the log and the scanner.
132+
133+
---
134+
7135
## 🚨 START HERE - Adding/Updating Sensors 🚨
8136

9137
**BEFORE making ANY changes to sensors or registers:**
@@ -18,6 +146,13 @@ This document provides comprehensive guidelines for AI assistants (and developer
18146
□ Step 6: Run validation script: python3 validate_sensors.py --sensor <name>
19147
```
20148

149+
**Removing a sensor is not the reverse of this list.** Deleting the register is not enough:
150+
the sensor platform creates whatever the profile's **sensor set** lists, and a
151+
`hasattr()` condition cannot stop it if the attribute is a dataclass field (rule 6). Remove
152+
it from the sensor set, and clear any already-registered entity in `__init__.py` — that
153+
cleanup must not be gated on the inverter being connected, because `coordinator.data` is an
154+
empty placeholder during setup (rule 5).
155+
21156
### 2. **Validation Tools**
22157
```bash
23158
# Validate a specific sensor
@@ -695,6 +830,63 @@ When preparing a release:
695830
- Check all changed sensors work
696831
- Test with at least one real device if possible
697832

833+
### Release cadence
834+
835+
**Pre-release by default.** `gh release create vX.Y.Z --prerelease` keeps the previous
836+
stable marked Latest, so HACS does not offer it unless the user opted into betas. Promote
837+
to stable once a reporter confirms, or batch weekly.
838+
839+
**One exception — ship stable immediately when we broke it.** A regression that takes
840+
entities offline, corrupts data, or publishes a wrong-but-plausible value cannot wait for
841+
a weekly batch, and a pre-release does not reach the people already affected. v1.3.6 and
842+
v1.4.1 both qualified.
843+
844+
Pre-release convention: bump `manifest.json`, but leave the README badge at the last
845+
**stable** version.
846+
847+
### Pulling a release
848+
849+
If a release must be withdrawn:
850+
851+
```bash
852+
gh release edit vX.Y.Z --draft # off the public list and out of HACS
853+
```
854+
855+
The previous stable becomes Latest again and the git tag survives. **If you then delete
856+
the tag, the draft is orphaned and does not disappear on its own** — find and delete it
857+
explicitly:
858+
859+
```bash
860+
gh api repos/OWNER/REPO/releases # find the draft's id
861+
gh api -X DELETE repos/OWNER/REPO/releases/ID
862+
```
863+
864+
Re-cut with `gh release create vX.Y.Z --target <sha>` so the tag matches the notes it
865+
describes. Never leave a published tag pointing at a commit whose notes have since changed.
866+
867+
### Verify after releasing
868+
869+
```bash
870+
git fetch origin --tags
871+
gh api repos/OWNER/REPO/git/ref/tags/vX.Y.Z # must equal main HEAD
872+
```
873+
874+
`git merge-base` and `rev-parse` give false negatives until you `git fetch --tags` — a tag
875+
created by `gh release create` exists only server-side until then.
876+
877+
---
878+
879+
## PowerShell Notes (Windows dev environment)
880+
881+
The shell is PowerShell 5.1. Bash idioms fail in ways that waste turns:
882+
883+
- **No heredocs.** `<<'EOF'` is a parser error. For multi-line commit messages write the
884+
message to a file and use `git commit -F <file>`.
885+
- **No `&&` or `||`.** Use `;` or `if ($?) { ... }`.
886+
- **Never write file content** — see rule 7 above. Edit/Write tools only.
887+
- `Get-Content -Raw` misreads UTF-8. Use `Grep` or Python to check for encoding damage.
888+
- `gh api` takes one positional arg; `--jq` with extra args fails confusingly.
889+
698890
---
699891

700892
## Quick Reference: File Responsibilities
@@ -730,5 +922,5 @@ When preparing a release:
730922

731923
---
732924

733-
*Last updated: 2026-01-29*
734-
*Integration version: 0.2.7*
925+
*Last updated: 2026-08-09*
926+
*Integration version: 1.4.1*

0 commit comments

Comments
 (0)