errors: name the cause, not the symptom (WhatsApp aiohttp preflight, lossless-compaction wording, empty-session resume) + CONTRIBUTING convention - #111630
Open
Finn763 wants to merge 1 commit into
Conversation
…nvention Wave NousResearch#111128 (error messages name the proximate symptom instead of the cause). Three user-facing messages reported the wrong subsystem at the failure site: - whatsapp: a half-removed aiohttp stays importable (namespace shell, no ClientSession), so every health probe raised AttributeError inside a blanket except and connect() blamed the Node bridge with "Bridge HTTP server did not start in 15s" forever. Preflight now names the broken dependency and the fix (pip install --force-reinstall aiohttp) (NousResearch#71308). - agent: the repeated-compaction warning claimed "accuracy may degrade" even for engines whose compaction keeps turns retrievable (LCM-style). Engines can declare ContextEngine.lossless_compaction = True and the message then says what actually happened (NousResearch#53000). - cli: resuming a session with an empty transcript printed "found but has no messages. Starting fresh." — on the preload path it actually aborted with exit 1. Both paths now name the empty transcript and the way out (hermes sessions delete <id>) (NousResearch#27168). CONTRIBUTING.md gains the convention: every user-facing error names the actual cause plus the remediation step, never the proximate symptom; regression test on the touched path. Tests: assertions added/updated on all three paths.
This branch has not been deployed
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.
Summary
Part of #111128 (wave: "Error messages lie — one convention plus a sweep"). A bounded slice: three user-visible messages that named a proximate symptom now name the real cause and the way out, plus the convention section the issue asks to land.
Every message below was verified unclaimed (
gh search prs --state open <issue#>+ the issue's keyword search + timeline/triage comments) before it was touched.1. #71308 — WhatsApp bridge blamed for a broken Python dep
adapter.connect()reported "Bridge HTTP server did not start in 15s" forever while the bridge was healthy: a half-removedaiohttpleft an importable namespace shell whose missingClientSessionraisedAttributeErrorinside three blanketexcept Exceptionhandlers.Now a preflight names the cause and the fix —
"aiohttp is installed but unusable: (namespace package: no __init__.py on disk) has no ClientSession — a half-removed or partially overwritten install. The bridge was never reached. Reinstall it: pip install --force-reinstall aiohttp"(separate text for the not-installed case), fatal codewhatsapp_aiohttp_unusable,retryable=False, checked before node/bridge/creds. The genuine bridge-timeout string is kept for real bridge failures. This also stops the useless 15 s poll.2. #53000 — lossless compaction told the user its accuracy degraded
agent/conversation_compression.pyprinted "ContextEngine.lossless_compaction = True(mirroring the existingemit_automatic_compaction_statusconvention) and the line becomes "ℹ️ Session compacted N times — this engine keeps compacted turns retrievable, so nothing was lost. /new only if you want a clean slate." Read viatype(compressor), so mock engines and the base engine keep today's wording.3. #27168 — resuming an empty session lied about "Starting fresh"
_preload_resumed_sessionaborts with exit 1, so "Session found but has no messages. Starting fresh." was false on that path. Both paths now state the empty transcript and give the command: "…its transcript is empty (nothing was committed to it, or the messages were cleared)… delete the empty one with:hermes sessions delete <id>". Exit codes and return values untouched.4. Convention
CONTRIBUTING.mdgains "Error Messages: Name the Cause, Not the Symptom" (one paragraph): every user-facing error must name the actual cause plus the next step, never the proximate symptom; suppressed exceptions must carry their reason into the final message; add a regression test on the touched path. Examples cite #105150 (payment/credit vs missing key) and this commit's aiohttp-vs-bridge case.Evidence
git show HEAD:<path>, test files kept): 5 failed / 2 passed across the three test files — the three new preflight cases, the lossless-engine case, and the updated resume assertion.tests/gateway/test_whatsapp_connect.py tests/agent/test_compression_count_warning.py tests/hermes_cli/test_resume_quiet_stderr.py tests/agent/test_413_compression.py tests/gateway/test_whatsapp_stale_bridge.py tests/gateway/test_telegram_noise_filter.py→ 197 passed, 0 failed, 2 skipped.Scope note
Three message fixes, not five: the wave's own list was swept first and all 10 listed children are claimed by open PRs or already fixed on main — #105150 (#64146), #96416 (#96437), #99831 (#99840), #53639 (#78045), #25087 (#24513/#52545), #85172 (#102591), #31791 (#31801/#106948), #65101 (#65128), #65099 (#65106); #70908 is fixed on main (
cron/scheduler.pygates provider classification onprovider_reachable). The three here come from the same cause-naming family and were each unclaimed. Behaviour changed: strings and the aiohttp guard only — no i18n, no logging overhaul.