Skip to content

✨ Standardize error message prefixes to [@umpire/package] format#44

Merged
sdougbrown merged 2 commits intomainfrom
claude/update-module-notation-Vzuw0
Apr 14, 2026
Merged

✨ Standardize error message prefixes to [@umpire/package] format#44
sdougbrown merged 2 commits intomainfrom
claude/update-module-notation-Vzuw0

Conversation

@sdougbrown
Copy link
Copy Markdown
Owner

  • Create error.ts helpers in core, json, reads, and signals packages
  • Update all throw statements to use standardized [@umpire/...] prefix
  • Add missing prefixes to previously un-prefixed errors (reads circular dependency, signals unknown fields)
  • Update tests to check error message content without hardcoding prefix format

Resolves #34: Inconsistent error message prefixes across packages

https://claude.ai/code/session_01BfgcHxYmkkBT2DUCKiMz8D

- Update all error messages in @umpire/core from [umpire] to [@umpire/core]
- Update all error messages in @umpire/json from [umpire/json] to [@umpire/json]
- Update error messages in @umpire/reads from [reads] to [@umpire/reads]
- Add [@umpire/reads] prefix to previously unprefixed circular dependency error
- Add [@umpire/signals] prefix to previously unprefixed unknown field errors
- Update tests to check message content without hardcoding prefix format

Resolves #34

https://claude.ai/code/session_01BfgcHxYmkkBT2DUCKiMz8D
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 14, 2026

Confidence Score: 4/5

  • Safe to merge — all error messages emit the correct prefix text today; the incomplete conversion is a style gap, not a behavioural regression.
  • The approach is sound and the vast majority of throw sites are correctly migrated. The 7 unconverted sites in umpire.ts, rules.ts, and expr.ts bypass the new err() helper but already carry the right hardcoded prefix text, so runtime behaviour is unchanged. This is a cleanup PR with no logic changes, and the test strategy improvement (substring matching) is a net positive. A single targeted follow-up can finish the remaining conversions.
  • packages/core/src/umpire.ts, packages/core/src/rules.ts, and packages/json/src/expr.ts each have throw sites that still call new Error(...) directly despite importing err.

Important Files Changed

Filename Overview
packages/core/src/error.ts New internal helper that wraps new Error() with the [@umpire/core] package prefix — simple, correct, not exported publicly.
packages/json/src/error.ts Same thin err() helper pattern as core, scoped to [@umpire/json].
packages/reads/src/error.ts New err() helper for [@umpire/reads]; also adds a previously missing prefix to the circular-dependency error in reads.ts.
packages/signals/src/error.ts New err() helper for [@umpire/signals]; reactive.ts now uses it for all three previously un-prefixed or manually-prefixed throws.
packages/core/src/umpire.ts Imports err and converts most throws, but four call sites (lines 229, 814, 827, 848) were missed and still call new Error('[umpire] ...') directly.
packages/core/src/rules.ts Imports err and converts most throws, but one call site (line 1264) still calls new Error('[umpire] ...') directly.
packages/json/src/expr.ts Imports err and converts most throws, but two fieldInCond call sites (lines 176, 185) still call new Error('[umpire/json] ...') directly.
packages/core/tests/validation.test.ts Correctly removes the hardcoded prefix from .toThrow() assertions — the substring matching strategy is resilient to future prefix changes.
packages/reads/tests/reads.test.ts Correctly removes the old [reads] prefix from .toThrow() assertion; circular-dependency test already used a plain substring so no change needed there.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    subgraph CORE["@umpire/core"]
        CE["error.ts — err()"]
        CE --> F["field.ts"]
        CE --> G["graph.ts"]
        CE --> R["rules.ts"]
        CE --> U["umpire.ts"]
    end
    subgraph JSON["@umpire/json"]
        JE["error.ts — err()"]
        JE --> B["builders.ts"]
        JE --> CO["check-ops.ts"]
        JE --> EX["expr.ts"]
        JE --> PA["parse.ts"]
        JE --> ST["strategies.ts"]
        JE --> VA["validate.ts"]
    end
    subgraph READS["@umpire/reads"]
        RE["error.ts — err()"]
        RE --> RD["reads.ts"]
    end
    subgraph SIGS["@umpire/signals"]
        SE["error.ts — err()"]
        SE --> RC["reactive.ts"]
    end
    U -. "7 unconverted sites still use new Error directly" .-> WARN["⚠ bypass"]
    style CE fill:#d4f5d4
    style JE fill:#d4f5d4
    style RE fill:#d4f5d4
    style SE fill:#d4f5d4
    style WARN fill:#fff3cd
Loading

Comments Outside Diff (1)

  1. packages/core/src/umpire.ts, line 229-231 (link)

    P2 Missed err() conversions in files that already import it

    umpire.ts, rules.ts, and expr.ts all import err from ./error.js as part of this PR, but several throw sites were not converted. They still hardcode the prefix string directly and will drift from the helper if the prefix ever changes.

    Locations with unconverted new Error(...) calls:

    • packages/core/src/umpire.ts:229Named field builder "..." does not match field key "..."
    • packages/core/src/umpire.ts:814Contradictory rules: "..." disabled whenever "..." is satisfied
    • packages/core/src/umpire.ts:827Contradictory rules: "..." oneOf(...) places them in different branches
    • packages/core/src/umpire.ts:848Unknown field "..." in oneOf(...) branch "..."
    • packages/core/src/rules.ts:1264Unknown active branch "..." for oneOf("...")
    • packages/json/src/expr.ts:176"fieldInCond" requires an array condition, but "..." is "..."
    • packages/json/src/expr.ts:185Runtime condition "..." must be an array for "fieldInCond"

    For example, umpire.ts:229 should be:

    The same pattern applies to the other six sites listed above.

    Fix in Claude Code Fix in Codex

Fix All in Claude Code Fix All in Codex

Reviews (1): Last reviewed commit: "✨ Standardize error message prefixes to ..." | Re-trigger Greptile

@sdougbrown sdougbrown force-pushed the claude/update-module-notation-Vzuw0 branch from 5cac8ca to 94e2bea Compare April 14, 2026 03:51
@sdougbrown sdougbrown merged commit 6060d47 into main Apr 14, 2026
8 checks passed
@sdougbrown sdougbrown deleted the claude/update-module-notation-Vzuw0 branch April 16, 2026 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Inconsistent error message prefixes across packages

2 participants