From 66cd3efacc01af9c203e63e7a776bbdbacfbd217 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Sun, 4 Oct 2026 17:37:42 -0700 Subject: [PATCH] fix(code-health): fix texts for HX010, HX001 and BLE001 say what is accepted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit unsupportedpastels M4 (review 5408300628 ยง2.3) and m7: the printed fixes would break code or did not match the engine. - HX010 led with `is_truthy_value`, which returns False for every real key, while most hits are "is it set" checks (`bool(os.getenv("..._API_KEY"))`). It now offers `is not None` / `!= ""` for presence and `is_truthy_value` only for boolean flags. - HX001 only said `get_hermes_home()`, which would move a path that lives under the user's home on purpose (the profiles root); such a path takes an allow with a reason. - BLE001: the ratchet runs ruff with --ignore-noqa, so `# noqa: BLE001` waives nothing, and a message-only `logger.warning(..., e)` still fails. Verified with pinned ruff 0.15.10 through the real comparison path (test_ble001_fix_text_matches_the_engine): logger.exception, any log call with exc_info=True, and re-raise pass; `% e` logging and `# noqa` block; `# health: allow BLE001 -- why` waives. Text only: no rule's blocking flag or ENFORCEMENT changed. --- scripts/code_health/config.py | 16 +++++++++++---- tests/scripts/test_code_health_rules.py | 27 +++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/scripts/code_health/config.py b/scripts/code_health/config.py index c860d3da03..680081b2d1 100644 --- a/scripts/code_health/config.py +++ b/scripts/code_health/config.py @@ -58,8 +58,12 @@ RULES: tuple[Rule, ...] = ( # Swallowed exceptions: the top statically-catchable bug cause in the fix sample. Rule("S110", "try/except/pass", "log it or narrow the except; a silent pass hides the" " failure the next bug report is about", "ruff"), + # ruff (pinned, --ignore-noqa) passes a handler that re-raises or logs the traceback; a + # message-only log such as `logger.warning("...: %s", e)` still counts as swallowed. Rule("BLE001", "blind `except Exception`", "catch the exceptions this code can actually" - " raise; if a boundary truly needs a catch-all, log it with context", "ruff"), + " raise; a deliberate catch-all boundary must re-raise or log the traceback" + " (`logger.exception(...)` or `exc_info=True`; `logger.warning(..., e)` is not enough)." + " `# noqa` is ignored here: waive with `# health: allow BLE001 -- `", "ruff"), Rule("E722", "bare `except:`", "catch a named exception (bare except also swallows" " KeyboardInterrupt/SystemExit)", "ruff"), # Correctness basics. @@ -89,7 +93,9 @@ RULES: tuple[Rule, ...] = ( " `gather(return_exceptions=True)` also returns CancelledError", "ast"), # Profile scope (one process serves many profiles). Rule("HX001", "hardcoded Hermes home", "`get_hermes_home()` for paths," - " `display_hermes_home()` for user-facing text (`hermes_constants`)", "ast", + " `display_hermes_home()` for user-facing text (`hermes_constants`); a path that" + " deliberately lives under the user's home, not the profile (e.g. the profiles root)," + " takes `# health: allow HX001 -- `", "ast", exclude=SINGLE_PROFILE + ("hermes_constants.py",)), Rule("HX002", "new HERMES_* environment variable", "behavioural settings go in" " config.yaml, secrets through the secret scope; `.env` is for credentials only", @@ -114,8 +120,10 @@ RULES: tuple[Rule, ...] = ( " `gateway.status.looks_like_gateway_command_line` /" " `hermes_cli.update_cmd._hermes_holder_subcommand` and match full cmdlines", "ast"), # Config truthiness. - Rule("HX010", "bool() of a config/env string", "`bool(\"false\")` is True: parse it" - " (`is_truthy_value`) or compare explicitly", "ast"), + Rule("HX010", "bool() of a config/env string", "`bool(\"false\")` is True. Is it set?" + " `os.getenv(\"X\") is not None` (or `!= \"\"` to treat empty as unset). A boolean" + " flag? `is_truthy_value(...)`, which is False for any non-flag string such as a key", + "ast"), # Structure. Rule("HX011", "if/elif ladder on one name", "use a dict/table -> handler (`_SLASH_DISPATCH`" " is the shape)", "ast", exclude=()), diff --git a/tests/scripts/test_code_health_rules.py b/tests/scripts/test_code_health_rules.py index f99ca82f8e..74ee2f1fe3 100644 --- a/tests/scripts/test_code_health_rules.py +++ b/tests/scripts/test_code_health_rules.py @@ -335,3 +335,30 @@ def test_raw_thread_context_from_another_function_is_not_accepted(): src = ("import contextvars\nimport threading\n\n\ndef g():\n ctx = contextvars.copy_context()\n" " return ctx\n\n\ndef f(fn, ctx):\n return threading.Thread(target=ctx.run)\n") assert _hits("HX012", src) == [11] + + +# --- fix texts say what the engine actually accepts --- + +def test_fix_texts(): + hx010 = RULES_BY_ID["HX010"].fix + assert "is not None" in hx010 and "is_truthy_value" in hx010 + assert hx010.index("is not None") < hx010.index("is_truthy_value") + assert "allow" in RULES_BY_ID["HX001"].fix + ble = RULES_BY_ID["BLE001"].fix + assert "noqa" in ble and "health: allow BLE001" in ble and "exc_info=True" in ble + + +_HANDLER = "import logging\n\nlogger = logging.getLogger(__name__)\n\n\ndef boundary():\n try:\n pass\n{}" + + +@pytest.mark.parametrize("handler, blocks", [ + (" except Exception:\n logger.exception('boundary failed')\n", False), + (" except Exception:\n logger.warning('boundary failed', exc_info=True)\n", False), + (" except Exception as exc:\n raise RuntimeError('boundary failed') from exc\n", False), + (" except Exception as exc:\n logger.warning('boundary failed: %s', exc)\n", True), + (" except Exception: # noqa: BLE001\n logger.warning('boundary failed')\n", True), + (" except Exception: # health: allow BLE001 -- top-level boundary\n logger.warning('x')\n", False), +]) +def test_ble001_fix_text_matches_the_engine(tmp_path, capsys, handler, blocks): + code, out = _judge(tmp_path, capsys, {}, {"pkg/h.py": _HANDLER.format(handler)}) + assert code == (1 if blocks else 0), out