Skip to content

PDX-485: chore(mcp) — shared warningCodes.ts enum#182

Open
mrdailey99 wants to merge 2 commits into
developfrom
chore/PDX-485-warning-codes-enum
Open

PDX-485: chore(mcp) — shared warningCodes.ts enum#182
mrdailey99 wants to merge 2 commits into
developfrom
chore/PDX-485-warning-codes-enum

Conversation

@mrdailey99
Copy link
Copy Markdown
Collaborator

Context

Part of the Provar MCP thread-prep work (PDX-485). Six sibling thread PRs (validation, properties, automation, RCA, JUnit, parallel-mode tuning) will each surface warning text. Today each thread emits ad-hoc prefixes (WARNING:, [provarHome], WARN —), producing inconsistent output for downstream AI agents and making cross-tool "Did you mean...?" guidance hard to standardise.

This is the small, focused infrastructure PR that lands first so the six sibling PRs can import from a single source.

Why

  • Stable warning surface. One enum, one formatter — sibling PRs can't drift on prefix or punctuation.
  • Cross-thread typo guidance. formatWarning(code, message, suggestion?) standardises the Did you mean '<suggestion>'? suffix.
  • Minimal surface area, zero call-site churn. No existing code is rewired in this PR — that's intentional. Sibling thread PRs adopt at their own pace without merge conflicts here.

What's in the diff

  • src/mcp/utils/warningCodes.tsWARNING_CODES const (PROVARHOME-001, DATA-001, PARALLEL-001, SCHEMA-001, RUN-001, JUNIT-001), WarningCode type, formatWarning(code, message, suggestion?).
  • test/unit/mcp/utils/warningCodes.test.ts — code-value parity table, formatWarning with and without suggestion, empty-string suggestion edge case.
  • docs/mcp.md — new "Warning codes" reference table + TOC entry. Per-row meanings are intentionally placeholders that subsequent thread PRs will refine as each surface stabilises.

Out of scope (per the coordination plan)

  • No package.json / server.json version bump (handled in a sweeper PR).
  • No existing tool is rewired to use the new enum. Sibling thread PRs do that.

Test plan

  • yarn compile — clean
  • yarn lint — clean (incl. script-name lint)
  • yarn test:only (bypassed wireit cache via node_modules/.bin/nyc node_modules/.bin/mocha "test/**/*.test.ts") — 1159 passing, 0 failing
  • New unit tests cover every enum value, both formatter branches, and the empty-suggestion edge case

Refs: PDX-485

…-thread feedback codes

RCA: Six sibling Provar MCP thread PRs (validation, properties, automation, RCA, JUnit, parallel-mode tuning) each independently emit warnings with ad-hoc string prefixes — `WARNING:`, `[provarHome]`, `WARN —` — producing inconsistent surface area for AI agents downstream and making cross-tool typo guidance ("Did you mean...?") harder to standardise. PDX-485 wants a single canonical enum the threads can import, so warning codes are coined once, formatted once, and documented once.

Fix: Adds src/mcp/utils/warningCodes.ts exporting WARNING_CODES (PROVARHOME-001, DATA-001, PARALLEL-001, SCHEMA-001, RUN-001, JUNIT-001), a WarningCode type derived from the enum, and a formatWarning(code, message, suggestion?) helper that emits `WARNING [<CODE>]: <message>` and appends ` Did you mean '<suggestion>'?` when a suggestion is provided. No call sites are touched in this PR — surface area is intentionally minimal so the six sibling thread PRs can import and adopt without merge conflicts. docs/mcp.md gains a new "Warning codes" reference table linked from the table of contents; per-row meanings are placeholders that subsequent thread PRs will refine.

Tests: New test/unit/mcp/utils/warningCodes.test.ts covers (1) each WARNING_CODES key maps to its expected wire string, (2) formatWarning without a suggestion returns the prefixed message exactly, (3) formatWarning with a suggestion appends the "Did you mean" suffix exactly, (4) an empty-string suggestion is treated as no suggestion. Validation: yarn compile clean, yarn lint clean, full mocha 1159 passing / 0 failing.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 19, 2026 20:38
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 19, 2026

Quality Orchestrator

🟢 LOW · 12 / 100 · Touches: utils. All changed files have mapped tests.


🧪 Tests to Run · Running 1 of 52 tests

  • unit/mcp/utils/warningCodes.test.ts
▶ Run command
npx vitest run \
  unit/mcp/utils/warningCodes.test.ts

⚡ quality-orchestrator  ·  /qo stub <file>  ·  qo analyze-local

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Introduces a shared MCP warning code registry and formatter to standardize warning output across upcoming “thread-prep” changes, plus documentation and unit coverage for the new surface.

Changes:

  • Added WARNING_CODES, WarningCode, and formatWarning() utility for consistent WARNING [CODE]: ... formatting (with optional “Did you mean …?” suffix).
  • Added unit tests covering code/value mappings and formatter behavior (with and without suggestion).
  • Documented the warning code table and warning message shape in docs/mcp.md.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/mcp/utils/warningCodes.ts Adds the canonical warning codes object, type, and formatter function.
test/unit/mcp/utils/warningCodes.test.ts Adds unit tests validating code strings and formatter output.
docs/mcp.md Adds a “Warning codes” section + TOC entry describing codes and output shape.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +14 to +28
const expected: Record<string, string> = {
PROVARHOME_001: 'PROVARHOME-001',
DATA_001: 'DATA-001',
PARALLEL_001: 'PARALLEL-001',
SCHEMA_001: 'SCHEMA-001',
RUN_001: 'RUN-001',
JUNIT_001: 'JUNIT-001',
};
for (const [key, value] of Object.entries(expected)) {
assert.equal(
(WARNING_CODES as Record<string, string>)[key],
value,
`WARNING_CODES.${key} should equal '${value}'`
);
}
…DES test

RCA: Copilot review on PR #182 flagged that the parity test only iterated the expected map and asserted forward-direction equality (expected key -> WARNING_CODES[key]). A new enum entry added without updating the test, or an accidentally-removed entry, would not fail the build. Bidirectional coverage was missing.

Fix: Hoist the expected map to the describe scope so it is reusable. Add two new assertions: (a) Object.keys(WARNING_CODES).sort() deepEqual Object.keys(expected).sort() — guards against additions and omissions in the key set; (b) Object.values(WARNING_CODES).sort() deepEqual Object.values(expected).sort() — guards against value drift. Tests now fail loudly on silent enum drift in either direction.
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.

2 participants