Skip to content

fix(ci): repair the lint gate and the CRLF-fragile source scan on main - #178

Open
fxogg06-jpg wants to merge 1 commit into
Rfym21:mainfrom
fxogg06-jpg:fix/ci-lint-and-crlf-source-scan
Open

fxogg06-jpg wants to merge 1 commit into
Rfym21:mainfrom
fxogg06-jpg:fix/ci-lint-and-crlf-source-scan

Conversation

@fxogg06-jpg

Copy link
Copy Markdown

Why

main at 928f816 has a red verify / regression job. The failing step is 7. Run bun run lint, which means step 8 (bun run test) is skipped — so the regression suite has not actually run on this commit. Fixing only the lint error would turn CI green while the second defect below stays hidden; both are included here.

Reproduced locally on a clean checkout of 928f816.

1. bun run lint fails with 4x no-useless-assignment

src/controllers/anthropic.js
  1265:7  error  The value assigned to 'promptTokens' is not used in subsequent statements
  1266:7  error  The value assigned to 'completionTokens' is not used in subsequent statements
  2044:7  error  The value assigned to 'promptTokens' is not used in subsequent statements
  2045:7  error  The value assigned to 'completionTokens' is not used in subsequent statements
✖ 4 problems (4 errors, 0 warnings)

In both handleAnthropicStream and handleAnthropicNonStream, the two counters were initialised to 0 and then unconditionally overwritten from the authoritative usage object:

let promptTokens = 0;
let completionTokens = 0;
...
const usage = reportUsage(upstreamUsage, ...);
promptTokens = usage.prompt_tokens;
completionTokens = usage.completion_tokens;

The initialisers are dead stores: nothing reads the variables before the assignment. The locals are write-only intermediates, so they are removed and both values are taken as const directly from usage at the single point where they are resolved.

No behaviour change — the values passed to attributeChatUsage and written into the emitted usage payload are byte-for-byte the same. This is a pure dead-store removal, not a change to how usage is resolved or estimated.

2. tests/tool-prompt.test.js:805 fails on any CRLF checkout

✖ lockstep: ningun sitio de prompt/hint re-ensena la forma nativa <tool_call>
  AssertionError: ../src/controllers/anthropic.js: cadena con la forma nativa
  <tool_call> legible por el modelo

That test scans chat.js / anthropic.js / request.js to pin at source level that no model-readable string re-teaches the native <tool_call> form. It deliberately strips comments first, because the rationale comments in the tree legitimately name <tool_call>:

const code = src
  .replace(/\/\*[\s\S]*?\*\//g, '')
  .split('\n')
  .map(line => line.replace(/\/\/.*$/, ''))   // <-- the bug
  .join('\n')

The per-line strip is anchored with $. On CRLF input . does not match \r, so $ matches before the CR, and because that regex has no g flag the engine backtracks to the empty match at position 0 — leaving the entire comment body in code, including the <tool_call> the test means to exclude. It then trips on its own rationale comments:

// 错误列表(hasParseError 永远为真,即使重试本身成功),而一个被截断的 <tool_call>

Bounding the strip by the newline instead is what the test intends, and behaves identically on LF:

.map(line => line.replace(/\/\/[^\r\n]*/, ''))

Also worth noting for triage: this is a git checkout dependency, not a platform one — core.autocrlf=true (default on Windows) is enough to trigger it, and the repo has no .gitattributes pinning * text=auto eol=lf. Adding one would be a more structural follow-up, but I kept this PR to the two defects so it stays reviewable.

Verification

Run on Windows, Bun 1.4.2 (from package.json), Node v24.21.0, matching the workflow's environment:

Gate Before After
bun run lint exit 1, 4 errors exit 0
bun run test never runs in CI (skipped after lint) PASS — 1146 tests / 134 suites / 0 fail
bun run build:frontend n/a exit 0
bun run test:bun n/a PASS (8/8)

1146 / 134 / 0 fail matches tests/expected-counts.json exactly, so the count gate is satisfied rather than merely non-failing. The smoke run covers real Bun startup, login, frontend, WASM, HTTP/SSE (including a 16 s upstream delay), and agent mid-stream quota failover.

Verified the patch applies cleanly onto a fresh clone of main at 928f816 before opening this PR.

Two independent defects broke the verification workflow on main:

1. `bun run lint` failed with 4x no-useless-assignment in
   src/controllers/anthropic.js. In handleAnthropicStream and
   handleAnthropicNonStream, `promptTokens`/`completionTokens` were
   initialised to 0 and then unconditionally overwritten from the
   authoritative usage object, so the initialisers were dead stores.
   Remove the write-only locals and take both values as consts directly
   from `usage` at the single point where they are resolved. No behavior
   change: the values consumed by attributeChatUsage and the emitted
   usage payload are identical.

2. tests/tool-prompt.test.js:805 failed on any CRLF checkout ("ningun
   sitio de prompt/hint re-ensena la forma nativa <tool_call>"). The
   per-line comment strip used a line-comment regex anchored with `$`,
   and on CRLF input the dot stops before the CR so `$` matches there;
   the match is not global, so the engine backtracked, matched the empty
   string, and left the comment body in `code`. The rationale comments
   that legitimately name `<tool_call>` then tripped the source scan.
   Bound the strip by the newline instead, which is what the test
   intends on both LF and CRLF checkouts.

Verified locally: lint exit 0; frontend build exit 0; test gate PASS
1146 tests / 134 suites / 0 fail (matches tests/expected-counts.json);
bun test:bun smoke PASS (8/8).
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.

1 participant