docs(adr): correct five ADRs describing gaps the engine has since closed - #380
Merged
Conversation
… closed Accuracy defects found by an adversarial refutation pass over the ADR publication audit. These are STALE, not disclosure questions: the prose is wrong about the engine as it stands, so the fix is an edit and not a vault move. Keeping them separate matters because the two outcomes route to different people. 0105 -- a tense defect, and the worst-reading of the three. Its own heading says the gap is CLOSED by Phase 3a and the line below the block says so again, while every sentence between them stayed present tense: purge "do not decref", the attachment and its chunks "are retained past their last referrer", a purged-but-still-referenced attachment "is not yet reclaimed". A skimmer lands on the body and concludes the engine currently over-retains purged PHI attachments. It does not. Body moved to past tense with an explicit note that it is kept as the record of what was true before Phase 3a; no technical content changed. 0111 -- false on every particular, so rewritten rather than patched. It described _SCALAR_FIELDS as a twelve-key whitelist with no passthrough that SILENTLY DROPS seven named fields. Measured today: thirty entries, and an unrecognised key is REJECTED fail-loud with a WiringError mirroring the loader's _reject_unknown shape, not dropped. Three of the seven named fields are now in the list. Both the count and the mechanism were wrong, which is why patching the number would have left the sentence false. The replacement carries no count at all -- it points at the constant, because a number written into an ADR is a number that goes stale. 0021 -- superseded direction, at THREE sites rather than the one first reported. It directed that connection_event.reason be flagged plaintext at rest on SQL Server, joining the message_events.detail / messages.error set. That direction was not followed and is no longer true: reason is encrypted on SQL Server too, on a composite-AAD pass rather than the id-keyed loop, because the table's IDENTITY id is unknown at INSERT so the cell AAD binds insert-time-known natural columns instead. The store's own code comment names this ADR section as stale. A NOTE FOR THE NEXT READER, because it cost a wrong conclusion here first: searching for _CIPHER_COLUMNS alone does NOT find the SQL Server treatment, and returns zero. That zero is true and answers a different question -- it is a fact about one mechanism name, not about whether the column is encrypted. The edit says so, so the next person does not repeat it. SITE COUNTS ARE THE TRAP IN ALL THREE. The audit rows cited one or two locations each; 0021 carries the stale claim at three (the DDL comment, the encryption-at-rest section, and the consequences summary). Editing only the cited site leaves the claim asserted elsewhere in the same file, which is the strike-the-banner-not-the-claim shape. The two remaining STALE ADRs from the same pass are 0030 and 0047, and 0047's claim is recorded at four sites -- they are deliberately NOT in this commit. Verified: no ledger file touched. The cp1252 encodability check was run against the ADDED lines only, which is the scope the no-glyph rule binds -- 33 added lines, 11 non-ASCII characters seen and none cp1252-unsafe. The 54 arrow characters elsewhere in these files are pre-existing and were left alone; scanning whole files would have reported them as mine. No banner flip and no BACKLOG citation: these ADRs were dispatched as an accuracy fix, not as a numbered item, and authoring ledger content is not the builder's.
…futation pass
Completes the five accuracy defects the pass identified. Same class as the previous
commit: the prose describes gaps the engine has closed, so the fix is an edit and not a
vault move.
0047 -- FOUR passages, not the two the audit row cited. It asserted that the built
[logging] syslog forwarder is plaintext and that SyslogProtocol has no TLS variant, and
built a decision on that premise: pair it with a TLS sidecar or collector, or keep
off-box forwarding out of the cloud manifest entirely.
MEASURED: SyslogProtocol.TLS exists and ships. config/settings.py documents it as RFC
5425, syslog over an ssl-wrapped TCP socket, native and needing no local agent, added
under ADR 0080, with the same down-at-startup-skipped and bounded-timeout posture as
tcp and a bounded handshake. __main__.py branches on SyslogProtocol.TLS.
All four passages corrected: the deployment note, the out-of-scope list, the To-resolve
checkbox (now closed, with the original text retained as the record of why it was
open), and the decisions summary. Editing only the two cited sites would have left the
claim asserted in the other two -- and the checkbox is the one that matters most,
because an open checkbox reads as outstanding work.
0030 -- FIVE sites across two mechanisms, and the audit row named one. It stated that
irreversibility is salt-dependent rather than cryptographic, that random.Random seeded
from a string is not a keyed hash, and that switching the seed derivation to an HMAC or
BLAKE2 keyed hash was an open To-resolve question.
MEASURED: that switch was made. messagefoundry/anon/keying.py uses a keyed BLAKE2b
under the per-dataset salt. The premise is false and the open question is answered.
The same stale mechanism was also asserted twice more in the preceding paragraph
("re-identification key for the seeded PRNG", "Keying is one-way (seeded PRNG, no
inverse)"), which the row did not name. Corrected there too.
KEPT DELIBERATELY: every operational rule the old paragraph stated is unchanged and
still binding -- the salt is never persisted or logged, and an (original, surrogate)
pair must never leak alongside dataset_key. The premise changed; the discipline did
not, and dropping it while fixing the premise would have been a real weakening.
NOT CHANGED: line 11's random.Random reference. It describes the synthetic HL7
GENERATORS, a different subsystem, and it is accurate -- generators/_core.py carries 24
such sites. Two mechanisms with one name in one file is exactly the shape that invites
a fix to the wrong one.
SITE COUNTS, AGAIN: the audit cited one or two locations per ADR; the real counts here
were four and five. Every one of the five stale ADRs in this pass understated its own
site count. That is worth carrying into any future doc-accuracy work -- the row tells you
where the reader looked, not where the claim lives.
Verified: no ledger file touched, no engine code touched. cp1252 encodability run against
the ADDED lines only, which is the scope the no-glyph rule binds -- 30 added lines, 1
non-ASCII character seen and none unsafe. Pre-existing arrow characters elsewhere in these
files were left alone.
No banner flip and no BACKLOG citation: dispatched as an accuracy fix, not a numbered
item, and authoring ledger content is not the builder's.
wshallwshall
enabled auto-merge (squash)
August 13, 2026 23:42
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Corrects five ADRs whose prose describes gaps the engine has since closed.
Accuracy defects, not disclosure questions. The fix is an edit, and the two outcomes route to
different people, so they are kept apart deliberately.
WiringError. Rewritten, and the replacement deliberately carries no count at all.SyslogProtocol.TLSships (RFC 5425, ADR 0080).The finding worth more than the five fixes
Every one of the five understated its own site count. The audit cited 1 or 2 locations each; the real
counts were 3, 4 and 5.
The sharpest instance: 0047's uncited site was a To-resolve CHECKBOX. A checkbox reads as outstanding
work regardless of what the prose above it says -- so fixing only the cited paragraphs would have left the
ADR still advertising an open item that is closed.
Two things kept deliberately, so they are not read as misses
and an
(original, surrogate)pair must never leak alongsidedataset_key. The premise changed; thediscipline did not. Dropping those while fixing the premise would have been a real weakening dressed
as an accuracy fix.
random.Randomreference stays. It describes the synthetic HL7 generators, adifferent subsystem, and it is accurate -- 24 such sites in
generators/_core.py. Two mechanismssharing a name in one file is what invites a fix to the wrong one.
No banner flip, and that is compliance rather than an oversight
Dispatched as an accuracy fix rather than a numbered item, and authoring ledger content is not the
builder's. Zero files under
docs/outsidedocs/adr/; no BACKLOG citation.Verification, scope stated
cp1252 encodability was run against the ADDED LINES ONLY, which is the scope the no-glyph rule binds:
68 added lines, zero cp1252-unsafe characters, confirmed with a positive control (fires on U+2192) and
a negative control (clean on ASCII). Pre-existing arrow characters elsewhere in these files were left
alone -- a whole-file scan reports 54 and would have attributed them to this change.