Skip to content

fix(quality): clear base-red lint + mutation-coverage gates on the release tip#7739

Closed
diegosouzapw wants to merge 1 commit into
release/v3.8.49from
fix/lint-any-and-stryker-base-red
Closed

fix(quality): clear base-red lint + mutation-coverage gates on the release tip#7739
diegosouzapw wants to merge 1 commit into
release/v3.8.49from
fix/lint-any-and-stryker-base-red

Conversation

@diegosouzapw

Copy link
Copy Markdown
Owner

Two quality gates fail on a pristine release/v3.8.49 checkout (zero modifications), blocking every open PR:

  • npm run lint320 @typescript-eslint/no-explicit-any errors across 5 test files that landed 2026-07-18/19 and were never added to eslint-suppressions.json.
  • npm run check:mutation-test-coverage — 4 covering unit tests missing from stryker.conf.json tap.testFiles.

Fix — real types, not suppression (per maintainer decision):

  • tests/_cp_mock_module.mts, base-executor-sanitize-effort, call-log-provider-display, combo-routing-engine, m365-web-token-extraction-7078 — each any replaced with a genuine structural type derived from the module under test. No eslint-disable, no suppression entry, no assertion changed — every test still passes (combo-routing 81/81, base-executor 36/36).
  • stryker.conf.json — added the 4 missing tap.testFiles entries the gate printed.

One frozen-file rebaseline: combo-routing-engine.test.ts 3243→3266 (+23 lines are the new type declarations; documented in file-size-baseline.json). lint now reports 0 errors; check:mutation-test-coverage passes.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request replaces any type annotations with real, derived types across several test files to resolve @typescript-eslint/no-explicit-any lint violations, registers missing test files in stryker.conf.json, and updates the file size baseline. The review feedback identifies potential TypeScript compilation errors introduced by these annotations: specifically, directly accessing properties on optional fields in SanitizedEffortBody under strictNullChecks (which should use optional chaining), and parameter contravariance violations under strictFunctionTypes when typing the target parameter as ResolvedTestTarget instead of SingleModelTarget in handleSingleModel callbacks.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +127 to +128
assert.equal((result as SanitizedEffortBody).reasoning.effort, "max");
assert.equal((result as SanitizedEffortBody).reasoning.summary, "auto");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Since reasoning is defined as an optional property in the SanitizedEffortBody interface, directly accessing .effort and .summary on it will trigger compilation errors under strictNullChecks (e.g., Object is possibly 'undefined'). Use optional chaining (?.) to safely access these nested properties.

  assert.equal((result as SanitizedEffortBody).reasoning?.effort, "max");
  assert.equal((result as SanitizedEffortBody).reasoning?.summary, "auto");

const result = sanitizeReasoningEffortForProvider(body, "claude", "claude-opus-4-6", null);
assert.equal(result, body, "high is supported — body returned unchanged");
assert.equal((result as any).output_config.effort, "high");
assert.equal((result as SanitizedEffortBody).output_config.effort, "high");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Since output_config is defined as an optional property in the SanitizedEffortBody interface, directly accessing .effort on it will trigger compilation errors under strictNullChecks. Use optional chaining (?.) to safely access this nested property.

  assert.equal((result as SanitizedEffortBody).output_config?.effort, "high");

Comment on lines +333 to 336
handleSingleModel: async (body: Record<string, unknown>, modelStr: string, target?: ResolvedTestTarget) => {
calls.push({
modelStr,
trafficType: target?.trafficType ?? "production",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Typing target as ResolvedTestTarget here violates parameter contravariance since HandleSingleModel expects SingleModelTarget. Under strictFunctionTypes, this will cause a compilation error. Additionally, at runtime, target can be { modelAbortSignal: AbortSignal } (which lacks kind: "model") if wrapped by the timeout runner. It is safer and more correct to type target as SingleModelTarget and narrow it inside the callback using a type guard.

    handleSingleModel: async (body: Record<string, unknown>, modelStr: string, target?: SingleModelTarget) => {
      const resolvedTarget = target && "kind" in target && target.kind === "model" ? target : undefined;
      calls.push({
        modelStr,
        trafficType: resolvedTarget?.trafficType ?? "production",

Comment on lines +441 to 442
handleSingleModel: async (_body: Record<string, unknown>, modelStr: string, target?: ResolvedTestTarget) => {
calls.push({ modelStr, trafficType: target?.trafficType ?? "production" });

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Typing target as ResolvedTestTarget here violates parameter contravariance since HandleSingleModel expects SingleModelTarget. Under strictFunctionTypes, this will cause a compilation error. Additionally, at runtime, target can be { modelAbortSignal: AbortSignal } (which lacks kind: "model") if wrapped by the timeout runner. It is safer and more correct to type target as SingleModelTarget and narrow it inside the callback using a type guard.

    handleSingleModel: async (_body: Record<string, unknown>, modelStr: string, target?: SingleModelTarget) => {
      const resolvedTarget = target && "kind" in target && target.kind === "model" ? target : undefined;
      calls.push({ modelStr, trafficType: resolvedTarget?.trafficType ?? "production" });

Comment on lines +835 to 836
handleSingleModel: async (_body: Record<string, unknown>, modelStr: string, target: ResolvedTestTarget) => {
calls.push(`${modelStr}:${target?.connectionId || "none"}`);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Typing target as ResolvedTestTarget here violates parameter contravariance since HandleSingleModel expects SingleModelTarget. Under strictFunctionTypes, this will cause a compilation error. Additionally, at runtime, target can be { modelAbortSignal: AbortSignal } (which lacks kind: "model") if wrapped by the timeout runner. It is safer and more correct to type target as SingleModelTarget and narrow it inside the callback using a type guard.

    handleSingleModel: async (_body: Record<string, unknown>, modelStr: string, target?: SingleModelTarget) => {
      const resolvedTarget = target && "kind" in target && target.kind === "model" ? target : undefined;
      calls.push(`${modelStr}:${resolvedTarget?.connectionId || "none"}`);

Comment on lines +2251 to 2252
handleSingleModel: async (_body: Record<string, unknown>, modelStr: string, target: ResolvedTestTarget) => {
calls.push({ modelStr, connectionId: target?.connectionId });

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Typing target as ResolvedTestTarget here violates parameter contravariance since HandleSingleModel expects SingleModelTarget. Under strictFunctionTypes, this will cause a compilation error. Additionally, at runtime, target can be { modelAbortSignal: AbortSignal } (which lacks kind: "model") if wrapped by the timeout runner. It is safer and more correct to type target as SingleModelTarget and narrow it inside the callback using a type guard.

    handleSingleModel: async (_body: Record<string, unknown>, modelStr: string, target?: SingleModelTarget) => {
      const resolvedTarget = target && "kind" in target && target.kind === "model" ? target : undefined;
      calls.push({ modelStr, connectionId: resolvedTarget?.connectionId });

@diegosouzapw

Copy link
Copy Markdown
Owner Author

Subsumed — both base-reds this PR targeted are already green on release/v3.8.49: the 320 no-explicit-any lint violations are cleared on the tip (the 5 targeted test files now lint with 0 errors) and the covering stryker tests are registered (commit 07b2cf9). This PR is now DIRTY/conflicting against the tip; closing as redundant.

@diegosouzapw
diegosouzapw deleted the fix/lint-any-and-stryker-base-red branch July 19, 2026 21:00
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