Skip to content

fix(dashboard): repair two typecheck regressions failing the gate on base - #11200

Closed
pacocartones wants to merge 1 commit into
cheaper-inference:release/v3.8.50from
pacocartones:fix/dashboard-typecheck-gate-red-on-base
Closed

pacocartones wants to merge 1 commit into
cheaper-inference:release/v3.8.50from
pacocartones:fix/dashboard-typecheck-gate-red-on-base

Conversation

@pacocartones

Copy link
Copy Markdown
Contributor

The gate is red on release/v3.8.50 itself

check:dashboard-typecheck (step #9 Quality gates (all, non-fail-fast) of Fast Quality Gates) fails on a pristine, unmodified checkout of the base branch. Every open PR against release/v3.8.50 inherits the failure through no fault of its own.

Reproduction on an untouched tree

Fresh clone at 2dd20331, zero local modifications:

$ git log -1 --format="%H %s"
2dd20331a7d8d2649d34988c07e22738a0762d2d feat(providers): add Logfare as a free OpenAI-compatible provider (#10644) (#10987)

$ git status --porcelain
(no output — clean)

$ npx tsc --pretty false --noEmit -p tsconfig.typecheck-dashboard.json

Diffed against the frozen baseline using the gate's own exported parseTscOutput / diffAgainstBaseline:

regressions: 2
  ✗ src/app/(dashboard)/dashboard/combos/page.tsx TS2554 (baseline 0, live 187)
  ✗ src/app/(dashboard)/dashboard/providers/[id]/components/HarImportButton.tsx TS2339 (baseline 0, live 1)

(CI reports 188 rather than 187 for TS2554; 9689dce removed one call site after those runs started. Same defect.)

Control group

Of the open PRs against release/v3.8.50 authored by different contributors, 10 of 10 fail this identical gate: #11194, #11189, #11188, #11187, #11182, #11179, #11178, #11177, #11175, #11166. None of them touch src/app/(dashboard)/.


Cause 1 — combos/page.tsx, 187 × TS2554

6cd4d38 added a 4th parameter to getI18nOrFallback():

-function getI18nOrFallback(t, key, fallback) {
+function getI18nOrFallback(t, key, fallback, values) {
   try {
-    if (typeof t.has === "function" && t.has(key)) return t(key);
+    if (typeof t.has === "function" && t.has(key)) return t(key, values);

The parameter is required, but only one of the 188 call sites was updated to pass it. All 187 errors are the same message, and every one lands on a getI18nOrFallback call:

187x  Expected 4 arguments, but got 3.
call sites landing on getI18nOrFallback: 187/187

Fix: make values optional. This is the shape the helper already assumes — it forwards values straight into t(key, values), and undefined is exactly what a 3-argument call meant before the change. No call site is rewritten and no behaviour changes.

Cause 2 — HarImportButton.tsx, 1 × TS2339

HarImportButton.tsx(72,57): error TS2339: Property 'error' does not exist on type 'M365HarImportResult'.

M365HarImportResult is a boolean-literal discriminated union. Under this repo's strict: false, !result.ok does not narrow such a union. Minimal repro with the same flags (--strict false --target ES2022):

type R = { ok: true; apiKey: string } | { ok: false; error: string };
declare const result: R;
if (!result.ok) { const e = result.error; }
// error TS2339: Property 'error' does not exist on type 'R'.

result.ok === false narrows correctly under the same flags (verified clean). Fix: use the explicit comparison. Semantically identical, no runtime change.


Verification

Full diff is 2 source lines:

-function getI18nOrFallback(t, key, fallback, values) {
+function getI18nOrFallback(t, key, fallback, values?) {

-    if (!result.ok) {
+    if (result.ok === false) {

Gate result after the fix:

regressions: 0

Nothing else moved. Only the two target files changed, and every baselined error is untouched:

total errors: pristine=417 -> fixed=229

CHANGED src/app/(dashboard)/dashboard/combos/page.tsx
   pristine {"TS2554":187,"TS2339":4,"TS2345":5,"TS2698":1,"TS2322":13}
   fixed    {"TS2339":4,"TS2345":5,"TS2698":1,"TS2322":13}
CHANGED .../HarImportButton.tsx
   pristine {"TS2339":1}
   fixed    {}

Dashboard test suite, including the HAR-import tests, on this branch:

ℹ tests 120
ℹ pass 120
ℹ fail 0

The frozen baseline is deliberately left untouched, per the gate's own instructions — these are genuine regressions, so widening it would defeat the gate.


Scope: this does not make the shards green

Being explicit so this is not oversold. The affected PRs fail 6 checks, and this PR only addresses the typecheck gate inside Fast Quality Gates. At least one other independent base failure exists, which I did not fix here:

✖ combo create inserts a new combo via db module
✖ combo delete removes the combo
✖ combo switch updates active combo when server is offline

Those live in tests/unit/cli-combo-command.test.ts (AssertionError: 1 !== 0), are unrelated to src/app/(dashboard)/, and also reproduce off a clean base. No new ESLint warnings is likewise still failing. Happy to open a separate issue or PR for those if useful.

…base

`check:dashboard-typecheck` fails on a pristine, unmodified checkout of
`release/v3.8.50`, so every open PR against that branch is red through no
fault of its own.

Two regressions, neither covered by the frozen baseline:

- `combos/page.tsx` TS2554 (baseline 0, live 187): `getI18nOrFallback()`
  gained a required 4th `values` parameter, but only one of its 188 call
  sites passes it. Made the parameter optional, which is the shape the
  helper already assumes -- it forwards `values` straight to `t(key, values)`
  and `undefined` is exactly what a 3-argument call meant before.

- `HarImportButton.tsx` TS2339 (baseline 0, live 1): `!result.ok` does not
  narrow the `M365HarImportResult` boolean-literal discriminated union under
  this repo's `strict: false` config, so `result.error` is unresolved.
  Switched to `result.ok === false`, which narrows correctly. No runtime
  behaviour change.

The baseline is deliberately left untouched, per the gate's own instructions.
@pacocartones

Copy link
Copy Markdown
Contributor Author

Superseded by 79c5bdf (fix(release): repair v3.8.50 base-red tail after latest root lift (#10964)), which landed on release/v3.8.50 at 02:09 UTC while I was verifying this branch. Closing to keep the queue clean.

That commit fixes both regressions, and independently arrived at the same conclusions:

  • combos/page.tsx — values = undefined (mine used values?; equivalent)
  • HarImportButton.tsx — result.ok === false (character-identical)

Verified against the gate on a clean checkout of the new base 79c5bdf68:

$ git log -1 --format="%h %s"
79c5bdf68 fix(release): repair v3.8.50 base-red tail after latest root lift (#10964)

$ npx tsc --pretty false --noEmit -p tsconfig.typecheck-dashboard.json
regressions: 0

Leaving the analysis in the PR body up, since it documents the root cause and the pristine-base reproduction if this class of regression recurs.

One note that may still be worth acting on: the check:dashboard-typecheck gate is not the only base-red. On a clean base, tests/unit/cli-combo-command.test.ts also fails (AssertionError: 1 !== 0) for combo create / combo delete / combo switch, which is what the Unit Tests fast-path shards trip over, and No new ESLint warnings is failing too. If 79c5bdf's "base-red tail" was not meant to cover those, I'm happy to open a separate issue with the reproduction.

@pacocartones

Copy link
Copy Markdown
Contributor Author

Following up on the offer I made when closing this PR: I said I would report the rest of the base red I ran into. It turned out to be fixable, so it is coming as PRs instead of an issue.

Two, both Windows-only, both reproduced on a clean checkout of release/v3.8.50 (0b41259f3):

  • fix(cli): resolve dynamic imports to file:// URLs so the DB fallback works on Windows #11238 — a real product bug. import() resolves its specifier as a URL, so interpolating an absolute path works by accident on POSIX but fails on Windows, where the drive letter parses as the URL scheme e:. bin/cli/runtime.mjs does this for the DB fallback modules, which means every CLI command that falls back to direct DB access when the server is offline exits 1 on Windows: combo list, create, delete and switch. Same pattern in setup.mjs and serve.mjs. tests/unit/cli-combo-command.test.ts goes 1/5 → 5/5.

  • test(cli): make the CLI test suite pass on Windows #11240 — tests only, no production change. tests/unit/cli/** reports 9 failures on Windows on a clean checkout: 2 in alias-resolver-7791 from the same URL-vs-path confusion inside the test, and 7 in run-command where the assertions compare raw plan.args while win32 deliberately escapes argv for cmd.exe (CVE-2024-27980 / DEP0190). The escaping is correct; the assertions were platform-specific. 170/182 → 179/182, zero failures.

The two branches are independent and were each verified standalone on 0b41259f3, so they can land in either order.

No action needed on this closed PR. Just closing the loop on what I said I would do.

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