refactor: tighten check_no_magic_numbers for named module constants (#1856)

CLAUDE.md

@@ -13,7 +13,7 @@ Web: see `web/CLAUDE.md`. CLI: see `cli/CLAUDE.md` (use `go -C cli`, never `cd c
 - **Persistence Boundary (MANDATORY)**: only `src/synthorg/persistence/` may import sqlite/psycopg or emit raw SQL. See [docs/reference/persistence-boundary.md](docs/reference/persistence-boundary.md).
 - **Convention Rollout (MANDATORY)**: every convention PR ships its enforcement gate. See [docs/reference/convention-gates.md](docs/reference/convention-gates.md).
 - **Configuration Precedence (MANDATORY)**: DB > env > YAML > code default via `SettingsService`/`ConfigResolver`; no `os.environ.get` outside startup. See [docs/reference/configuration-precedence.md](docs/reference/configuration-precedence.md).
-- **No Hardcoded Values (MANDATORY)**: numerics live in `settings/definitions/`; allowlist 0/1/-1, HTTP codes, hex masks, powers-of-2. Enforced by `scripts/check_no_magic_numbers.py`.
+- **No Hardcoded Values (MANDATORY)**: numerics live in `settings/definitions/`; allowlist 0/1/-1, HTTP codes, hex masks, powers-of-2, and module-level annotated named constants of the form `NAME: int|float|Final|Final[int]|Final[float] = literal`. Enforced by `scripts/check_no_magic_numbers.py`.
 - **Doc Numeric Claims (MANDATORY)**: numerics in README + public docs sourced from `data/runtime_stats.yaml` via `<!--RS:NAME-->` markers. See `data/README.md`.
 - **Test Regression (MANDATORY)**: timeout/slow failures = source-code regression; never edit `tests/baselines/unit_timing.json` or any `scripts/*_baseline.{txt,json}` / `scripts/_*_baseline.py`. Both families are PreToolUse-blocked. Per-invocation bypass for gate baselines: `ALLOW_BASELINE_GROWTH=1 git commit ...` (requires explicit user approval).
 - **Post-Implementation + Pre-PR Review (MANDATORY)**: after issue: branch + commit + push (no auto-PR); use `/pre-pr-review` (gh pr create is hookify-blocked). After PR: `/aurelio-review-pr` for external feedback. Fix EVERYTHING valid; no deferring.

docs/reference/configuration-precedence.md

@@ -128,6 +128,26 @@ clean: there is exactly one env var name per setting.
    the env var via `os.environ` at startup (init-time).  Direct env
    reads in application code outside startup are forbidden.
 
+## Protocol constants are not settings
+
+Wire-protocol numerics such as JSON-RPC error codes
+(`JSONRPC_PARSE_ERROR: int = -32700`), framing thresholds, or
+specification-mandated limits are NOT operator-tunable policy: changing
+the value silently breaks interop with peers that read the public
+spec. Express them as typed module-level constants and let
+`scripts/check_no_magic_numbers.py` recognise the annotation as the
+named-constant signal. Examples:
+
+```python
+JSONRPC_PARSE_ERROR: int = -32700
+A2A_TASK_NOT_FOUND: int = -32001
+_MAX_FRAME_SIZE: Final[int] = 16384
+```
+
+Do not register these in `settings/definitions/`. The precedence
+chain is for values that an operator may legitimately tune; protocol
+constants are part of the algorithm.
+
 ## Migration path
 
 When a previously-direct env-var read needs to become a registry

scripts/check_no_magic_numbers.py

@@ -56,6 +56,12 @@
   generates these.
 - Files under ``src/synthorg/observability/events/`` -- event-name
   registries and version constants.
+- Module-level annotated numeric constants of the form
+  ``NAME: int|float|Final|Final[int]|Final[float] = literal`` -- the
+  annotation declares the literal IS the named constant the rule wants
+  to encourage, so flagging produces only noise. Bare ``NAME = literal``
+  without an annotation still flags; the developer must opt in by
+  typing the assignment.
 
 Per-line opt-out
 ----------------
@@ -122,6 +128,9 @@ def _baseline_path(project_root: Path) -> Path:
 # setting. The gate matches by *parameter name* in function signatures
 # whose default is a power-of-2; raw call-site arguments are not
 # scanned (the gate only looks at module-level assigns and defaults).
+# Locked by: test_io_default_allowlisted_all_kwargs_all_pow2 /
+#            test_io_kwarg_with_non_pow2_still_flagged /
+#            test_chunk_size_default_flags_at_all_pow2.
 _IO_KEYWORD_NAMES: Final[frozenset[str]] = frozenset(
     {
         "buffering",
@@ -138,16 +147,29 @@ def _baseline_path(project_root: Path) -> Path:
 # HTTP status code allowlist context. The literal is exempt when:
 #   - it's the value of a kwarg named ``status_code`` / ``status``
 #   - it's the default of a parameter named ``status_code`` / ``status``
+# Locked by: test_status_default_allowlisted_all_kwargs.
 _HTTP_STATUS_KEYWORDS: Final[frozenset[str]] = frozenset({"status_code", "status"})
 
 # Path-prefix allowlist for whole-file exemptions. Files under these
 # prefixes are skipped entirely. POSIX-relative.
+# Locked by: test_file_prefix_allowlist /
+#            test_file_prefix_allowlist_does_not_match_substring.
 _FILE_ALLOWLIST_PREFIXES: Final[tuple[str, ...]] = (
     "src/synthorg/settings/definitions/",
     "src/synthorg/persistence/migrations/",
     "src/synthorg/observability/events/",
 )
 
+# Module-level annotation shapes that mark a numeric named constant.
+# Locked by: test_named_constant_allowlist_contents +
+#            test_annotation_marks_as_named_constant_helper.
+_NAMED_CONSTANT_TYPE_NAMES: Final[frozenset[str]] = frozenset(
+    {"int", "float", "Final"},
+)
+_NAMED_CONSTANT_FINAL_SLICES: Final[frozenset[str]] = frozenset(
+    {"int", "float"},
+)
+
 # ── Helpers ─────────────────────────────────────────────────────
 
 
@@ -292,6 +314,33 @@ def _effective_value(node: ast.expr) -> float | None:
     return -value if negated else value
 
 
+def _annotation_marks_as_named_constant(annotation: ast.expr | None) -> bool:
+    """Return True iff *annotation* declares a numeric named constant.
+
+    The gate treats an annotated module-level assignment as the
+    developer's explicit declaration that the literal IS the named
+    constant; an unannotated assignment is ambiguous (could be a
+    one-time module-load computation) and continues to flag. Qualified
+    forms like ``typing.Final[int]`` are deliberately not matched --
+    direct ``Final`` imports are the project convention.
+    """
+    if annotation is None:
+        return False
+    if isinstance(annotation, ast.Name) and annotation.id in _NAMED_CONSTANT_TYPE_NAMES:
+        return True
+    if not isinstance(annotation, ast.Subscript):
+        return False
+    if not isinstance(annotation.value, ast.Name):
+        return False
+    if annotation.value.id != "Final":
+        return False
+    slice_node = annotation.slice
+    return (
+        isinstance(slice_node, ast.Name)
+        and slice_node.id in _NAMED_CONSTANT_FINAL_SLICES
+    )
+
+
 def _is_hex_literal(node: ast.expr, source_lines: list[str]) -> bool:
     """Return True iff *node*'s source spelling starts with ``0x``.
 
@@ -460,7 +509,11 @@ def _collect_module_assign_hits(
                 if not isinstance(target, ast.Name):
                     continue
                 hit = _classify_module_assign(
-                    target.id, assign_node.value, rel, source_lines
+                    target.id,
+                    assign_node.value,
+                    None,
+                    rel,
+                    source_lines,
                 )
                 if hit is not None:
                     hits.append(hit)
@@ -472,6 +525,7 @@ def _collect_module_assign_hits(
             hit = _classify_module_assign(
                 assign_node.target.id,
                 assign_node.value,
+                assign_node.annotation,
                 rel,
                 source_lines,
             )
@@ -505,13 +559,21 @@ def _hit_is_suppressed(hit: _Hit, source_lines: list[str]) -> bool:
     return _line_has_trailing_marker(source_lines[line_idx])
 
 
-def _classify_module_assign(
+def _classify_module_assign(  # noqa: PLR0911
     name: str,
     value_node: ast.expr,
+    annotation: ast.expr | None,
     rel: str,
     source_lines: list[str],
 ) -> _Hit | None:
-    """Module-level ``NAME = <number>`` -> hit if not allowlisted."""
+    """Module-level ``NAME = <number>`` -> hit if not allowlisted.
+
+    *annotation* is the PEP-526 annotation for ``ast.AnnAssign`` or
+    ``None`` for bare ``ast.Assign``. A numeric named-constant marker
+    short-circuits to ``None``; bare assignments still flag.
+    """
+    if _annotation_marks_as_named_constant(annotation):
+        return None
     inner, negated = _unwrap_unary(value_node)
     if not _is_numeric_constant(inner):
         return None