Skip to content

fix(ci): keep timed model-sync logs visible to the error-helper gate - #18711

Closed
HouMinXi wants to merge 2 commits into
cheaper-inference:release/v3.8.53from
HouMinXi:fix/model-sync-error-helper-timing
Closed

HouMinXi wants to merge 2 commits into
cheaper-inference:release/v3.8.53from
HouMinXi:fix/model-sync-error-helper-timing

Conversation

@HouMinXi

@HouMinXi HouMinXi commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Summary

The model-sync timing wrapper added in #18466 hides the final catch block's saveCallLog call from check:error-helper. The gate flags the internal audit entry even though the HTTP response still uses sanitizeErrorMessage.

The catch block now calls saveCallLog directly inside timings.measure("call-log", ...). The log fields, best-effort rejection handling and HTTP sanitizer stay unchanged. The timer now also includes construction of the log entry. No checker rules, allowlists, dependencies or environment settings change.

Related: #17219 and the CI investigation in #18658. This patch does not address the separate environment-documentation failure.

Local validation

Based on release/v3.8.53 at 139070680f77728e2309eb4df18463282a1c990a.

  • Added tests/unit/model-sync-error-helper.test.ts: five tests cover the real route, timed internal logging and raw HTTP negative controls. The real-route test fails before the fix and passes after it.
  • All 59 tests across that file and the three existing check-error-helper*.test.ts files pass with Node 24.22.1's native TypeScript support.
  • Syntax and whitespace checks pass.

Local checks used exact source files retrieved from GitHub, without a complete installed checkout. Project ESLint, Prettier, typecheck, hooks, the whole-repository gate and route integration tests were not run locally. Prettier and local hooks remain unverified.

CI after marking ready for review

These runs checked head 78ccd6387082f59f92fd819a7887d652d2769551 against base 139070680f77728e2309eb4df18463282a1c990a, using merge checkout 4e8ea35bdf423cd7e25ff75a2bdb908a5373c7fd.

Passing checks:

  • Error-helper check: check:error-helper passed across 1,405 files with the same 18 frozen exceptions. The containing Fast Quality Gates job failed on other checks listed below.
  • Unit shard 6: all five new error-helper regression tests and all seven existing sync-models-phase-timings.test.ts tests passed, including real-route persistence, completed call-log timing, disabled instrumentation and the invalid-discovery 502 path.
  • Build, the ESLint step, no-new-warnings check, security tests and Quality Ratchet passed.

CI, Quality Gates and API Route Typecheck still fail. The failures below are reported in files or checks this two-file patch does not modify:

  • Docs counts: seven strict drifts, covering provider counts (377 vs 380) and migration counts (213 vs 219). Unit shard 1 reports the same docs-count gate failure.
  • Environment docs: OMNIROUTE_MODEL_SYNC_TIMINGS is missing from .env.example; unit shards 3 and 7 fail the environment contract checks.
  • SSO script guard: unit shard 2 flags the file://${process.argv[1]} direct-run check in scripts/cli/omniroute-sso.mjs.
  • Generated skills: four generated skills are out of sync. Changelog integrity passed.
  • Fast Quality Gates: provider-asset provenance cannot find audited commit ca7fc3f5ff3ec3e1f10e056faeb7332a568f6b28; five covering test entries are missing from mutation configuration; the vulnerability count is 29 against baseline 27; dashboard typecheck reports cliTools.ts TS2353. Provenance also stops the Lint job after ESLint; the vulnerability count also fails Extended Quality.
  • API Route Typecheck: availabilityRouterFilters.ts TS2345 and cliTools.ts TS2353 exceed the frozen baseline.
  • Vitest UI suite: 4,067 passed, one skipped, one failed. provider-stats-error-state.test.tsx:43 expects the alert to disappear after dismissal, but it remains.

Unit shards 4, 5, 6 and 8 passed. Coverage was skipped after unit failures; aggregate CI and Quality admission failed. These runs do not establish that every remaining failure reproduces on the base alone, and the Vitest failure has not been classified as flaky. Merge readiness has not been verified.

Related baseline tracking: #17219.

@HouMinXi

HouMinXi commented Nov 1, 2026

Copy link
Copy Markdown
Contributor Author

Both files from this PR are now included unchanged in #18658, whose latest CI runs passed. Closing this duplicate to keep review in one place. #18658 remains open and awaits maintainer review.

@HouMinXi HouMinXi closed this Nov 1, 2026
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