Skip to content

fix: resolve reported reliability issues across FixMap - #82

Merged
aryamthecodebreaker merged 2 commits into
mainfrom
codex/fix-claude-issues-67-81
Jul 25, 2026
Merged

fix: resolve reported reliability issues across FixMap#82
aryamthecodebreaker merged 2 commits into
mainfrom
codex/fix-claude-issues-67-81

Conversation

@aryamthecodebreaker

Copy link
Copy Markdown
Owner

What changed

This batch verifies all 15 newly filed reports against current main and fixes the 14 that still reproduce:

  • makes subdirectory diff paths repository-relative and preserves successful remote reports when cleanup fails
  • fixes inflection stemming and adds compound-definition ranking coverage to keep evaluation above its gate
  • adds real ESLint coverage for core, CLI, and Action workspaces
  • isolates the Action smoke test from ambient GitHub credentials and output files
  • makes external evaluation caches commit-verified, atomic, and failure-safe
  • extracts testable CLI and Action runners; fixes version/invalid-value parsing and validates MCP arguments at runtime
  • makes PR comment upserts identity-independent by default and removes the ten-page pagination cap
  • uses random, newline-safe, single-write Action outputs and byte-safe step-summary truncation
  • replaces the website demo's copied ranker with the browser-safe core implementation
  • hard-wraps unbroken demo paths without looping
  • refreshes the Action bundle, workspace smoke reports, and patched PostCSS dependency

Issue #72 does not reproduce on current main: scripts/evaluate.mjs already multiplies raw rates by 100. This PR still improves the external evaluation failure message with measured values and floors; #72 will be closed separately as already resolved rather than claiming a nonexistent regression fix.

Verification

  • npm run ci
    • 149 tests pass (9 Action, 82 CLI/MCP, 58 core)
    • typecheck and ESLint pass in every workspace
    • CLI, Action, and workspace smoke tests pass
    • Next.js production build passes
    • internal evaluation: top-1 62.5%, top-3 87.5% (gate: 50% / 80%)
    • 1,000-file benchmark passes
    • high-severity production audit gate passes
  • git diff --check

The latest MCP SDK still carries its upstream moderate @hono/node-server advisory; no high-severity production advisory remains.

Closes #67
Closes #68
Closes #69
Closes #70
Closes #71
Closes #73
Closes #74
Closes #75
Closes #76
Closes #77
Closes #78
Closes #79
Closes #80
Closes #81

`npm ci` failed on CI with:

    Missing: @emnapi/runtime@1.11.2 from lock file
    Missing: @emnapi/core@1.11.2 from lock file

Regenerating package-lock.json on Windows drops these optional-peer
entries, which npm ci on Linux requires. Restored both blocks verbatim
from origin/main; @emnapi/wasi-threads had survived. Verified with
`npm ci --dry-run`, which now resolves cleanly.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@aryamthecodebreaker

Copy link
Copy Markdown
Owner Author

CI is green again — the failure was the @emnapi lockfile regression, fixed in f290da7 (details in that commit message).

I reviewed the substantive changes against the 15 issues. The fixes for #67 (git diff --relative), #68 (cleanup as a warning diagnostic + cause preservation), #71 (allowlisted env + asserting on GITHUB_OUTPUT/GITHUB_STEP_SUMMARY), #80 (hex delimiter, trailing-newline guard, summary byte cap), and #81 (wrapLine always makes progress) all look correct to me.

One regression to flag, in the #69 stemmer fix

The new normalizeToken fixes every case I filed, and correctly handles doubled consonants — but it introduces a new mismatch for four-letter bases ending in e.

Run against packages/core/src/signals.ts on this branch:

failed/fail        fail vs fail             MATCH
boxes/box          box vs box               MATCH
routing/route      rout vs rout             MATCH
parsing/parse      pars vs pars             MATCH
created/create     creat vs creat           MATCH
invoices/invoice   invoic vs invoic         MATCH
stopped/stop       stop vs stop             MATCH
cached/cache       cach vs cach             MATCH
coding/code        cod  vs code             MISMATCH   <-
based/base         bas  vs base             MISMATCH   <-
filed/file         fil  vs file             MISMATCH   <-
sized/size         siz  vs size             MISMATCH   <-
dated/date         dat  vs date             MISMATCH   <-
timed/time         tim  vs time             MISMATCH   <-

filed/file and dated/date match on main today, so those are regressions rather than pre-existing gaps. The affected bases — base, code, file, size, date, time, line, mode, page, case, role, site, rule — are about as common as task vocabulary gets in this domain.

Cause: normalizeTrailingE guards on token.length > 4, so a four-letter base keeps its e, while its -ed/-ing form stems to a three-letter root that has no e left to strip.

function normalizeTrailingE(token: string): string {
  return token.length > 4 && token.endsWith("e") ? token.slice(0, -1) : token;
}

Candidate fix, and the tradeoff it carries

Relaxing the guard to token.length > 3 makes all 14 pairs above MATCH. I verified that directly. But it is not free, so it is your call rather than something I wanted to push:

  1. Stop-word collisions. note -> not and here -> her, both of which are in STOP_WORDS, so those tokens get dropped entirely. note is unfortunate given RiskNote / "risk notes" is FixMap's own vocabulary. (core, page, role, line are unaffected — they stem to cor, pag, rol, lin.)
  2. The -s rule returns early. if (token.length > 3 && token.endsWith("s")) return token.slice(0, -1); skips normalizeTrailingE, so under a > 3 guard notes -> note but note -> not, and the singular/plural pair stops matching. That rule would need to route through normalizeTrailingE too.

Whichever way it goes, npm run evaluate is the arbiter — worth recording the before/after top-1 and top-3 in the PR description, since this changes many token values (every word longer than four characters ending in e now loses it).

A round-trip assertion in signals.test.tsstem(inflected) === stem(base) over a table of pairs, rather than asserting literal token strings — would keep the -ed, -ing, -es, -s, and trailing-e rules from disagreeing again. The table above is a ready-made set of cases.

Two minor notes

  • packages/core/test/render-demo-wrap.test.mjs and external-cache.test.mjs test helpers that live in scripts/. It works because core's vitest run picks them up, but it means packages/core's suite fails when a build script changes. A scripts/test/ project in the vitest config would keep the layering honest.
  • The postcss 8.5.16 -> 8.5.23 bump is unrelated to the 15 issues. Fine by me, just worth a line in the PR description so it is not a surprise at release time.

@aryamthecodebreaker
aryamthecodebreaker marked this pull request as ready for review July 25, 2026 04:15
@aryamthecodebreaker
aryamthecodebreaker merged commit b1990da into main Jul 25, 2026
2 checks passed
aryamthecodebreaker added a commit that referenced this pull request Jul 25, 2026
)

#82 fixed the -ed/-es under-stripping from #69 but regressed a different
class: four-letter bases ending in e. normalizeTrailingE guarded on
length > 4, so file kept its e while filed stemmed to fil, and the
pair stopped matching. Five pairs that matched before #82 broke:
base/based, file/filed, date/dated, size/sized, time/timed.

Relaxing the guard to > 3 converges them. The result is still three
characters, which is the minimum token length tokenizeText keeps, and the
-s path is unaffected: a word ending in s but not es cannot leave a
trailing e behind.

Cost: note and here now stem onto the stop words not and her and
are dropped. That is the second stop-word filter working as designed, and
it buys consistency for base, code, file, size, date, time, line, mode,
page and role.

Extends the round-trip table in signals.test.ts from 5 to 18 pairs so the
-ed, -ing, -es, -s and trailing-e rules cannot drift apart again.

Ranking is unchanged: npm run evaluate holds at top-1 62.5% / top-3 87.5%,
and the only golden-report movement is the displayed stem code -> cod,
with identical scores, order and confidence.

Closes #83

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment