docs(hermes): §10's isolation recipe destroys the WDSP wisdom cache - #5417
Conversation
§10 tells harness writers to redirect HOME, CFFIXED_USER_HOME and XDG_CONFIG_HOME, and two bullets later promises that after the first ~19 s open "every later open is 40-175 ms". The second claim assumes the wisdom cache survives, and the first recipe is what destroys it. This is the harness-side half of aethersdr#5413's cause. The three variables move three different things and only two are about settings. WdspChannel::wisdomPath() resolves to $XDG_CACHE_HOME, else $HOME/.cache, then /aethersdr/wdsp-fftw-wisdom -- so a redirected HOME points it at an empty directory and each run writes its own cache into a temporary tree that is then deleted. Every run becomes a first open. Far worse than ~19 s, because WDSP builds every FFT with FFTW_PATIENT, 35 plans per channel. Measured on one machine and one binary, changing only whether the cache was reachable: 4.1 s present, still running at 150 s absent. The fix is one variable the code already provides for its own tests, AETHER_WDSP_WISDOM_DIR, pointed at an ABSOLUTE directory that outlives the run. The example is absolute deliberately and the text says why: a $HOME-relative path lands inside the temporary tree, because HOME is the variable this recipe redirects, so the fix deletes itself and the symptom is indistinguishable from never having applied it. On one command line with HOME=$T in front it appears to work, by luck of shell expansion order; in a script that sets HOME first it does not. (hl2-telemetry hit exactly this.) Two further traps are named because both cost a day here. There are TWO FFTW wisdom files, and the startup line "Audio NR2 wisdom summary: status=missing" refers to AudioEngine's, not WdspChannel's -- it is routinely absent and says nothing about the one that matters. And until aethersdr#5413 a stalled DSP open produced no diagnostic at all, so this presented as a hang rather than as slowness. Cited by symbol rather than line, so the paragraph does not rot the way the line citations in this file's neighbours have. Docs only: one file, no build, no behaviour change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CkisdWQiVxcUG6r6X9W3Fg
Ozy311
left a comment
Ozy311
left a comment
There was a problem hiding this comment.
Issue fit
This fits the PR's stated docs-only intent: it explains the WDSP-wisdom side of the slow pre-wire phase reported in #5413 and points harness authors at the existing AETHER_WDSP_WISDOM_DIR override. It does not implement #5413's timeout/log/connect-state requirements, and the body correctly calls it the harness-side half rather than claiming to close the issue. The core source claims about WdspChannel::wisdomPath(), eager import/export, the separate NR2 wisdom file, and the absolute-path shell-expansion trap are supported by current main. One central claim about the displayed §10 recipe's lifecycle is not.
Scope
| File | What it changes | Claimed by title/body? | Verdict |
|---|---|---|---|
docs/HERMES.md |
Adds WDSP-wisdom persistence guidance beside the isolated-profile recipe | Yes | In scope; factual blocker below |
No code, settings schema, protocol, UI, test registration, dependency, or CHANGELOG.md change is hidden in the diff.
Blockers
- [P3] The documented recipe does not delete its redirected HOME, so the headline and repeated “every run” explanation are false for the commands immediately above. Inline at
docs/HERMES.md:667. The recipe exports the stable path/tmp/aether-hl2-test, usesmkdir -p, and has no cleanup step;WdspChannel::wisdomPath()therefore writes$T/.cache/aethersdr/wdsp-fftw-wisdom, which the next invocation reuses. A separate harness that creates and removes a fresh profile can absolutely have the measured 150-second behavior, but that is a condition the shown recipe does not establish. Reframe the guidance conditionally for disposable-profile harnesses, or change the documented lifecycle so the new external cache is actually necessary.
Nits
- Non-blocking: the “three different things / only one ... settings” sentence contradicts both its table and the commit message; inline at
docs/HERMES.md:670. - Non-blocking:
#5413is the issue report, not a landed diagnostic; current main still lacks the timeout/log change and #5415 remains open with changes requested. Inline atdocs/HERMES.md:722.
What I tried to break
- Read the exact current-main implementation at
WdspChannel.cpp:50-84,138-208,822,860-861: the override wins first, the POSIX fallback is$XDG_CACHE_HOMEthen$HOME/.cache, wisdom is imported before planning and exported after a successful open. Those claims hold. - Read
AudioEngine::wisdomFilePath()and the summary logger: NR2 uses its distinct~/.config/AetherSDR/aethersdr_fftw_wisdompath, so the warning-disambiguation claim holds. - Walked the shell expansion cases. Exporting an absolute override before the inline
HOME=$T ...launch is sound, and a laterexport AETHER_WDSP_WISDOM_DIR=$HOME/...after changing HOME would point inside the isolated tree as warned. - Compared the PR merge base
b9f44f35with current main0ad8ca80: no intervening commit toucheddocs/HERMES.mdor the cited wisdom/settings/logging sources, andgit diff --checkis clean. - Did not run the app, GUI, hardware, sockets, or a build: this is documentation-only, and the 4.1 s / 150 s timing corpus is reported by the author rather than reproduced here. The commit signature is valid; this head has no CI runs.
Recommendation
Request changes. The operational workaround is sensible and the code citations mostly survive adversarial checking, but the paragraph currently diagnoses a cache-destruction behavior that the recipe directly above does not perform. Make the disposable-profile condition explicit (and clean up the two wording/status nits); then this should be a straightforward docs merge after the normal human CODEOWNERS review.
…actually costs you (aethersdr#5417 review). Principle X. The reviewer caught a claim that the page's own recipe disproves. I wrote "each run writes its own cache into a temporary tree that is then deleted -- so every run is a first open, for ever". The recipe directly above exports a stable $T=/tmp/aether-hl2-test and mkdir -p's it. Nothing removes it. Its cache therefore survives, and only the first launch is slow. The headline diagnosed a lifecycle the shown commands do not have. What is true, and is the part worth knowing: a redirected HOME puts the cache somewhere the operator's own cache is not, so the FIRST isolated run is a cold open even on a machine that has connected a hundred times. After that it depends on the profile's lifetime, which the harness author chooses -- stable directory, one slow launch; mktemp -d or a cleanup trap or a fresh container, no warm run ever. Both shapes present as the same symptom, so the text now says decide it rather than discover it. REPLACED "still running at 150 s" AS THE HEADLINE NUMBER. It was the moment an observer gave up, not a completion time, and quoting it as though it bounded the phase is what produced the 90 s timeout in aethersdr#5415 that failed working connects. hl2-telemetry has since measured the open itself: 98.3 s cold on an idle machine, 188.1 s at load 38-40, 86 ms warm. The table now carries both rows and says which one to quote and why, and names the repo the raw result lives in -- it is in the bench notebook, not in this tree, and a reader following the path from here would not have found it. Two nits from the same review: - The "three variables move three different things, and only one of them is about settings" sentence contradicted the table under it: two of the three move config locations and HOME moves those too. Says what the table says now. - "aethersdr#5413" was written as though it were a landed diagnostic. It is the report; aethersdr#5415 is the fix and is not merged. On current main the silence is still what a caller gets, and the text now says so. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CkisdWQiVxcUG6r6X9W3Fg
The reviewer caught a claim that the page's own recipe disproves. I wrote "each run writes its own cache into a temporary tree that is then deleted -- so every run is a first open, for ever". The recipe directly above exports a stable $T=/tmp/aether-hl2-test and mkdir -p's it. Nothing removes it. Its cache therefore survives, and only the first launch is slow. The headline diagnosed a lifecycle the shown commands do not have. What is true, and is the part worth knowing: a redirected HOME puts the cache somewhere the operator's own cache is not, so the FIRST isolated run is a cold open even on a machine that has connected a hundred times. After that it depends on the profile's lifetime, which the harness author chooses -- stable directory, one slow launch; mktemp -d or a cleanup trap or a fresh container, no warm run ever. Both shapes present as the same symptom, so the text now says decide it rather than discover it. REPLACED "still running at 150 s" AS THE HEADLINE NUMBER. It was the moment an observer gave up, not a completion time, and quoting it as though it bounded the phase is what produced the 90 s timeout in aethersdr#5415 that failed working connects. hl2-telemetry has since measured the open itself: 98.3 s cold on an idle machine, 188.1 s at load 38-40, 86 ms warm. The table now carries both rows and says which one to quote and why, and names the repo the raw result lives in -- it is in the bench notebook, not in this tree, and a reader following the path from here would not have found it. Two nits from the same review: - The "three variables move three different things, and only one of them is about settings" sentence contradicted the table under it: two of the three move config locations and HOME moves those too. Says what the table says now. - "aethersdr#5413" was written as though it were a landed diagnostic. It is the report; aethersdr#5415 is the fix and is not merged. On current main the silence is still what a caller gets, and the text now says so. Refs: PR aethersdr#5417, review round 2. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CkisdWQiVxcUG6r6X9W3Fg
|
Blocker fixed, at Both nits are also in. The three-variables sentence agreed with neither the table nor the commit message and now says what the table says. The "#5413 exposed it" wording is gone; it read as though the diagnostic exists today, and it does not. One substantive change beyond your review, because it was the same fault: the headline number was "still running at 150 s", which was the moment an observer gave up rather than a measurement. Quoting it as though it bounded anything is what produced the 90 s watchdog in #5415 that failed working connects. Replaced with measured figures and their conditions. |
b76db2f to
ff112c4
Compare
|
Head updated: subjects trimmed under the 72-character rule in |
![aethersdr-agent[bot]](https://avatars.githubusercontent.com/in/3286087?s=60&v=4){kind=small}
There was a problem hiding this comment.
Issue fit
No linked issue, and none is needed — this is documentation of an operational trap, which GOVERNANCE.md does not gate behind an issue or RFC. Reviewed against the PR's own stated intent. The mechanism it documents is real and I verified every code claim in it against the head checkout (details in "What I tried to break"). The second commit ff112c4 already addresses all three of @Ozy311's comments: the recipe-lifetime claim is now conditional and correctly describes $T=/tmp/aether-hl2-test as surviving between runs, the three-variables sentence now matches the table, and the #5413/#5415 attribution is now "reports that" / "is the fix and is not merged".
Scope
| file / group | what it changes | claimed by title/body? | verdict |
|---|---|---|---|
docs/HERMES.md §10 (+80 lines) |
one new bullet after the "~19 s first open" bullet: HOME → wisdom-cache table, cold/warm timings, AETHER_WDSP_WISDOM_DIR recipe, two traps |
yes | in scope |
No code, no CI, no build config, no CHANGELOG.md entry, no tests. Nothing else in the diff. No socket tests added or removed. No new public/protocol surface — AETHER_WDSP_WISDOM_DIR already exists at src/core/dsp/WdspChannel.cpp:64 and is already documented at docs/HERMES.md:362; this bullet documents an additional caller use, it does not introduce anything.
CodeGuard: all 11 CG-PATH-001 findings are in .github/workflows/, src/gui/MainWindow.cpp, src/gui/MemoryDialog.cpp and tests/local_control_server_test.cpp — none of which this PR touches. Dropping all of them as out-of-diff.
Blockers
None. This is documentation, the mechanism is verified, and nothing here changes behaviour.
Nits (non-blocking)
-
XDG_CACHE_HOMEbeatsHOME, and the bullet's conclusion doesn't survive that — see inline on 680–685. The doc states the resolution order correctly and then draws an unconditional conclusion from it. On a machine withXDG_CACHE_HOMEexported (common on Linux), redirectingHOMEmoves nothing, the isolated run is warm, and — becauseexportWisdomNow()runs unbounded in an app process (WdspChannel.cpp:860) — it also writes into the operator's real cache, which is the one thing the recipe's own comment at line 623 says it cannot do. Two sentences fix it. -
Two different cold-open numbers, forty lines apart — see inline on 696–703. The bullet directly above says ~19 s; this one says 98.3 s idle / 188.1 s loaded.
WdspChannel.cpp:157independently records "connect (21 s of FFTW planning)". The new bullet acknowledges the gap ("far worse than the ~19 s above") without reconciling it, so a reader gets two numbers and no rule for which applies. My guess is receiver count (the recipe setsreceiverCount:4, and 35 plans is per channel) but that is a guess and the doc should say. -
"Do not set a timeout against it" is in tension with #5415, by this PR's own numbers — see inline on 705–712. #5415 (same author) arms a 90 s fail stage over exactly this window. 98.3 s idle and 188.1 s under load both exceed it. Worth reconciling across the two PRs before either lands; the evidence in this file is the strongest argument that 90 s is too tight for a cold multi-receiver open.
-
Windows is silently excluded.
wisdomPath()usesLOCALAPPDATAunder#ifdef _WIN32(WdspChannel.cpp:71-72);HOMEdoes not reach the cache there at all. §10 is a macOS recipe so the practical impact is nil, but the table row reads as unconditional. -
The quoted log line is a paraphrase, not the literal string.
Audio NR2 wisdom summary: status=missingis emitted as two lines —"Audio NR2 wisdom summary:"then" context=%1 status=missing action=train-on-first-enable"(AudioEngine.cpp:6560-6593). A reader grepping the log for the quoted string finds nothing. SuggestAudio NR2 wisdom summary: … status=missing. -
The PR title now outlives the fix. After
ff112c4, the body correctly says the recipe's cache survives between runs. "destroys the WDSP wisdom cache" is the claim @Ozy311 refuted and it is still the title. -
The bench evidence lives in
hl2-lab, a repo no reader of this one can open. Fine to cite, but the operational instruction ("do not set a timeout") rests entirely on it — I could not check it, and neither can a future maintainer.
What I tried to break
Everything below is reasoned from reading the head checkout at /tmp/aetherclaude/pr-5417; I have no build and ran nothing.
- Every code assertion in the new text, against source.
WdspChannel::wisdomPath()(WdspChannel.cpp:50-85) —AETHER_WDSP_WISDOM_DIRreally is the first branch,XDG_CACHE_HOMEreally precedes$HOME/.cache, and the leaf really isaethersdr/wdsp-fftw-wisdom. "Imported before the channels are built and exported after" — confirmed,loadWisdomOnce()at:822andexportWisdomNow()at:860insideopen(). "FFTW_PATIENT, 35 plans per channel" — matches the comment at:43-44verbatim.AudioEngine::wisdomFilePath()under~/.config/AetherSDR/— confirmed atAudioEngine.cpp:6521-6530. All correct; the only thing I found wrong is the conclusion drawn from the resolution order, not the order itself. - "On current
mainthe silence is what you get." Held. I readbeginDspSetup()atHl2Backend.cpp:1952-2056and there is noQTimer, noqCInfo, noqCWarninganywhere in that window — only thedspSetupProgresssignal, whose one consumer isMainWindow. - The
$HOME-relative trap. I tried to find a case where a$HOME-relativeAETHER_WDSP_WISDOM_DIRstill works in a script; the doc's own distinction (one-lineVAR=x cmdprefix vs. a script that assignsHOMEfirst) is exactly right, and the "appears to work" framing is the honest one. - Whether the bullet duplicates or contradicts existing §10 text.
docs/HERMES.md:345-378already coversAETHER_WDSP_WISDOM_DIR— but for the test suite and viaTestWdspWisdomIsolation.cpp, a different audience and a different mechanism. Not duplication. The~19 sconflict in nit 2 is the one place where the new text and the old text disagree. - Scope and self-certification. One file, 80 added lines, zero deletions. No removed guards, no deleted comments, no
CHANGELOG.mdentry, nothing bundled. - What I could not check: the timing table. All four numbers come from
hl2-lab, which is not in this repo, and I cannot run a connect. Nit 3's tension with #5415 is arithmetic on the doc's own figures, not an independent measurement.
Recommendation
Approve with nits. The mechanism is real, the code references are accurate, and the previous round's review comments are genuinely addressed rather than papered over. Nit 1 is the one I would want fixed before merge — as written, the bullet tells a Linux operator with XDG_CACHE_HOME set the opposite of what will happen, in a section whose entire purpose is precision about which variable moves what. Nits 2 and 3 are worth a sentence each and are cheap. Everything else can ride.
Nice piece of documentation, and thank you for the "what this does not establish" honesty — flagging the abandoned 150 s observation as "not a completion time" is exactly the right instinct.
🤖 aethersdr-agent · cost: $5.4808 · model: claude-opus-5
|
Cisco CodeGuard — static analysis of this PR (11 finding(s))
Automated static scan by Cisco DefenseClaw CodeGuard on the changed files. Advisory — some may be false positives; the review above verifies them. 🤖 aethersdr-agent · cost: $5.6003 · model: claude-opus-5 |
…s conditional. Principle X. Upstream's non-blocking nit, verified against wisdomPath() rather than taken: XDG_CACHE_HOME is checked before HOME and the two are an if/else, so where XDG_CACHE_HOME is exported, redirecting HOME moves the wisdom cache nowhere. My bullet stated the resolution order correctly and then drew a conclusion that only holds on one branch of it -- "the first isolated run is a cold open" is false on any machine with XDG_CACHE_HOME set, which is common on Linux and rare on macOS. A harness would behave one way on a developer's box and another in CI. THE DANGEROUS OUTCOME IS THE FAST ONE. The export is unconditional in an app process, so such a run does not merely READ the operator's real cache, it WRITES to it -- fast, apparently correct, quietly rewriting a file outside the profile it was supposed to be confined to. That is now the headline of the bullet rather than the slowness. Also states why AETHER_WDSP_WISDOM_DIR is the fix rather than a convenience: it is checked first and unconditionally, ahead of all three platform branches, so it binds whatever the environment carries. Redirecting HOME is a guess about which branch a machine will take. AND THE GENERALISABLE HALF, which is ours rather than the reviewer's: verify the path the PROCESS got, not the one you asked for. A harness that exports a variable and prints that it exported it has confirmed its own intent, not the outcome; a request accepted, validated, reported fine and then discarded downstream produces identical output to one that worked. Read ps eww or /proc/<pid>/environ, or log wisdomPath() from inside the app and compare. We learned that the expensive way today, on a different mechanism with the same outcome. Refs: PR aethersdr#5417, review round 2. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CkisdWQiVxcUG6r6X9W3Fg
|
Re-review requested. Both of @Ozy311's rounds are answered, and I want to be precise about which part of that is confirmed by someone other than me. The Confirmed independently, not by me: @aethersdr-agent's review today states of
Checked against The bullet now leads with the consequence rather than the ordering, because the dangerous outcome is the fast one:
One clause is added that is not from the review, offered because it is the general form of the trap and it cost us a run to learn: verify the path the process actually got, not the one you asked for. A harness that exports a variable and prints that it exported it has confirmed its own intent, not the outcome; a request that is accepted, validated, reported fine and then discarded further down produces output identical to one that worked. CodeGuard's 11 findings: not addressed, deliberately. They are in Head is |
Ozy311
left a comment
Ozy311
left a comment
There was a problem hiding this comment.
Follow-up to my review of 347311dd59cf81837e414fc2d0a29971b3919afa, at ab58f5c187d5fc706690f8f084ab859d54088b87.
The original blocker and both wording nits are addressed: the text now distinguishes the stable /tmp/aether-hl2-test recipe from disposable profiles, explains the configuration variables without the earlier count claim, and says the diagnostic PR is still unmerged. The XDG_CACHE_HOME precedence matches src/core/dsp/WdspChannel.cpp.
| Scope | Follow-up result |
|---|---|
docs/HERMES.md |
Requested corrections are present; documentation-only scope |
No remaining blocker from my original review. One non-blocking consistency nit: the opening bullet still says a redirected HOME starts without a cache, and the table states that HOME moves the WDSP cache without qualification. The later XDG paragraph correctly explains the exception; qualifying the opener/table would keep readers who stop there from getting the opposite conclusion.
Verified this round: source/path-resolution inspection, unchanged current head, clean diff whitespace, and successful visible CI checks. The author's bench timings were not independently reproduced; no app, hardware, sockets, or TX were used. This is a COMMENT recording resolution of my earlier findings, not a merge approval.
Trusted governance refreshed at bc8017740de7cce1ddbc90bbb69bbfafa00ec44b: manifest PASS; constitution mirrors identical.
Ozy311
left a comment
Ozy311
left a comment
There was a problem hiding this comment.
The wisdom-cache guide now describes current source and the landed #5415 diagnostics accurately. No blockers remain.
| Scope | Claimed | Verdict |
|---|---|---|
| docs/HERMES.md | Cache isolation/lifetime and cold-open guidance | In scope, documentation only |
Verified cache precedence (explicit override, platform environment fallback), stable cache versus disposable settings lifetime, opening/chains-open diagnostics, 10-second warning / 30-second repetition / 600-second setup failure, and stale-attempt cleanup. Historical 19-second and 40–175 ms measurements are explicitly not current timeout estimates; the later 98/188-second observations are not explained away without evidence. These are source-verified documentation corrections; no bench timings were independently rerun and no new runtime coverage is claimed. All five checks pass at this exact head.
CODEOWNERS: all final changed paths are Tier 3 reviewers-owned. Ozy311 is an active reviewers member with write permission and the sole assignee. Signed commits verified, main 1b5f02b integrated; approving under the operator's direct repair and squash-merge authorization.
2 participants
What and why
This is the harness-side half of #5413's cause. #5415 bounds and logs the
DSP-setup phase; this explains why that phase takes minutes in a harness and
seconds otherwise.
§10 tells harness writers to redirect
HOME,CFFIXED_USER_HOMEandXDG_CONFIG_HOMEfor an isolated profile. Two bullets later it promises thatafter the first ~19 s open, "every later open — any receiver, any sample rate
— is 40–175 ms."
That second claim assumes the FFTW wisdom cache survives between runs, and the
recipe above it is what destroys it. The page contradicts itself, and the
reassuring half is the wrong one — which is why the paragraph goes directly
under that bullet rather than at the end of the section.
The mechanism
The three variables move three different things, and only two are about settings:
CFFIXED_USER_HOMEXDG_CONFIG_HOMEHOMEQDir::homePath()— and the WDSP wisdom cacheWdspChannel::wisdomPath()resolves to$XDG_CACHE_HOME, else$HOME/.cache,then
/aethersdr/wdsp-fftw-wisdom. A redirectedHOMEpoints it at an emptydirectory, and each run writes its own cache into a temporary tree that is then
deleted — so every run is a first open, for ever.
Far worse than ~19 s: WDSP builds every FFT with
FFTW_PATIENT, 35 plans perchannel. Measured on one machine and one binary, changing only whether that
cache was reachable:
The fix, and the way it silently undoes itself
One variable the code already provides for its own tests —
AETHER_WDSP_WISDOM_DIR— pointed at a directory that outlives the run.The example is absolute deliberately, and the text says why.
HOMEis thevariable this recipe redirects, so a
$HOME-relative path lands inside thetemporary tree and is deleted with it: the fix deletes itself, and the symptom
is indistinguishable from never having applied it. On a single command line with
HOME=$Tin front it appears to work, by luck of shell expansion order; in ascript that sets
HOMEfirst it does not.Two traps named because both cost a day
AudioEngine::wisdomFilePath()under~/.config/AetherSDR/for NR2, andWdspChannel::wisdomPath()under~/.cache/aethersdr/for the channels. The startup lineAudio NR2 wisdom summary: status=missingrefers to the first, isroutinely absent, and says nothing about the second.
line — so it presented as a hang rather than as slowness.
Citations
By symbol, not line —
WdspChannel::wisdomPath(),AudioEngine::wisdomFilePath(),QDir::homePath()— so the paragraph does notrot the way line citations in this file's neighbours have. Verified: three
symbols, zero line citations.
Validation
One commit, one file, +59, no deletions.
git diff --checkclean. No build:this changes no code.
🤖 Generated with Claude Code
https://claude.ai/code/session_01FtHEsQsghFwhUQEZdwVKUz