Skip to content

Strengthen bifurcation handler typing and align compartment/URL codec tests with actual guarantees#77

Merged
akutuva21 merged 5 commits into
mainfrom
copilot/refactor-bifurcation-analysis-type-safety
Apr 30, 2026
Merged

Strengthen bifurcation handler typing and align compartment/URL codec tests with actual guarantees#77
akutuva21 merged 5 commits into
mainfrom
copilot/refactor-bifurcation-analysis-type-safety

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 19, 2026

This PR applies the requested review diffs across the MCP bifurcation handler and related tests. It removes unsafe typing/casts, improves fallback observability, and updates tests/docs to validate the intended behavior more explicitly and deterministically.

  • Bifurcation handler: replace unsafe any with explicit types

    • Added a typed argument shape for handleBifurcationAnalysis inputs.
    • Replaced untyped dynamic import usage with a typed engine module contract plus runtime module-shape assertion.
    • Typed continuation/bifurcation result handling and removed rhsFn as any.
  • Bifurcation handler: make fallback failures observable

    • Replaced empty catch with a logged warning that preserves fallback behavior while surfacing JIT compilation root cause.
  • Compartment parity test: docs and parsing cleanup

    • Updated test docblock to match what is actually asserted (qualitative outcome + conservation, not quantitative rate-scaling verification).
    • Removed unnecessary nullish coalescing on execSync(..., { encoding: 'utf-8' }).
    • Corrected numeric parsing regex to reject invalid standalone decimal-point matches.
  • URL state codec tests: stronger, clearer, deterministic assertions

    • Tightened malformed params warnings assertions to explicitly detect both malformed segments (kf:notanumber and empty ,, segment).
    • Added fallback coverage for empty/missing tab values.
    • Replaced compact/deflate magic numbers with named constants.
    • Replaced Math.random() in round-trip test data with deterministic values.
type BifurcationAnalysisArgs = {
  code?: string;
  max_steps?: number;
  parameter?: string;
  start_value?: number;
  end_value?: number;
};

const parsedArgs: BifurcationAnalysisArgs = (args ?? {}) as BifurcationAnalysisArgs;

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • rest.uniprot.org
    • Triggering command: /opt/hostedtoolcache/node/24.14.1/x64/bin/node /opt/hostedtoolcache/node/24.14.1/x64/bin/node --experimental-import-meta-resolve --require /home/REDACTED/work/bngplayground/bngplayground/node_modules/vitest/suppress-warnings.cjs --conditions node --conditions development /home/REDACTED/work/bngplayground/bngplayground/node_modules/vitest/dist/workers/forks.js (dns block)
  • www.pathwaycommons.org
    • Triggering command: /opt/hostedtoolcache/node/24.14.1/x64/bin/node /opt/hostedtoolcache/node/24.14.1/x64/bin/node --experimental-import-meta-resolve --require /home/REDACTED/work/bngplayground/bngplayground/node_modules/vitest/suppress-warnings.cjs --conditions node --conditions development /home/REDACTED/work/bngplayground/bngplayground/node_modules/vitest/dist/workers/forks.js (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Original prompt
Please apply the following diffs and create a pull request.
Once the PR is ready, give it a title based on the messages of the fixes being applied.

[{"message":"The use of `as any` bypasses type safety. Define a proper interface for the expected argument structure to enable type checking and improve code maintainability.","fixFiles":[{"filePath":"packages/mcp-server/src/handlers/bifurcationAnalysis.ts","diff":"diff --git a/packages/mcp-server/src/handlers/bifurcationAnalysis.ts b/packages/mcp-server/src/handlers/bifurcationAnalysis.ts\n--- a/packages/mcp-server/src/handlers/bifurcationAnalysis.ts\n+++ b/packages/mcp-server/src/handlers/bifurcationAnalysis.ts\n@@ -2,8 +2,16 @@\n import { createToolResult, parseModelOrThrow, expandModel } from '../services/engine.js';\n import { structureError } from '../services/errors.js';\n \n+type BifurcationAnalysisArgs = {\n+  code?: string;\n+  max_steps?: number;\n+  parameter?: string;\n+  start_value?: number;\n+  end_value?: number;\n+};\n+\n export async function handleBifurcationAnalysis(args: ToolArgs): Promise<ToolResult<any>> {\n-  const parsedArgs = (args ?? {}) as any;\n+  const parsedArgs: BifurcationAnalysisArgs = (args ?? {}) as BifurcationAnalysisArgs;\n   try {\n     const engine = await import('@bngplayground/engine') as any;\n     const model = parseModelOrThrow(parsedArgs.code);\n"}]},{"message":"Dynamic import cast to `any` loses type information. Consider importing specific types from the engine package or creating a typed interface for the imported module.","fixFiles":[{"filePath":"packages/mcp-server/src/handlers/bifurcationAnalysis.ts","diff":"diff --git a/packages/mcp-server/src/handlers/bifurcationAnalysis.ts b/packages/mcp-server/src/handlers/bifurcationAnalysis.ts\n--- a/packages/mcp-server/src/handlers/bifurcationAnalysis.ts\n+++ b/packages/mcp-server/src/handlers/bifurcationAnalysis.ts\n@@ -2,10 +2,38 @@\n import { createToolResult, parseModelOrThrow, expandModel } from '../services/engine.js';\n import { structureError } from '../services/errors.js';\n \n+type EngineContinuationPoint = { stable: boolean };\n+type EngineBifurcation = {\n+  parameterValue: number;\n+  type: string;\n+  frequency?: number;\n+  criticalEigenvalues?: unknown[];\n+};\n+type EngineContinuationResult = {\n+  bifurcations: EngineBifurcation[];\n+  path: EngineContinuationPoint[];\n+};\n+type EngineModule = {\n+  JITCompiler: new (model: unknown) => {\n+    compile: () => {\n+      rhs: (y: Float64Array, dydt: Float64Array) => void;\n+    };\n+  };\n+  continuation: (args: {\n+    nSpecies: number;\n+    rhsFn: (y: Float64Array, p: number, dydt: Float64Array) => void;\n+    initialState: Float64Array;\n+    parameterStart: number;\n+    parameterEnd: number;\n+    stepSize: number;\n+    maxSteps: number;\n+  }) => Promise<EngineContinuationResult>;\n+};\n+\n export async function handleBifurcationAnalysis(args: ToolArgs): Promise<ToolResult<any>> {\n   const parsedArgs = (args ?? {}) as any;\n   try {\n-    const engine = await import('@bngplayground/engine') as any;\n+    const engine = (await import('@bngplayground/engine')) as EngineModule;\n     const model = parseModelOrThrow(parsedArgs.code);\n     const expandedModel = await expandModel(model);\n \n"}]},{"message":"Empty catch block swallows errors silently. Consider logging the error or at least adding a comment explaining why errors are intentionally ignored here.","fixFiles":[{"filePath":"packages/mcp-server/src/handlers/bifurcationAnalysis.ts","diff":"diff --git a/packages/mcp-server/src/handlers/bifurcationAnalysis.ts b/packages/mcp-server/src/handlers/bifurcationAnalysis.ts\n--- a/packages/mcp-server/src/handlers/bifurcationAnalysis.ts\n+++ b/packages/mcp-server/src/handlers/bifurcationAnalysis.ts\n@@ -45,7 +45,11 @@\n         compiled.updateParameters?.(params);\n         compiled.evaluate(0, y, dydt);\n       };\n-    } catch {\n+    } catch (jitError: any) {\n+      // Fallback is intentional, but log the root cause for observability/debugging.\n+      console.warn(\n+        `[bifurcationAnalysis] Failed to compile JIT RHS; falling back to zero RHS: ${jitError?.message ?? String(jitError)}`\n+      );\n       // Fallback: zero RHS (continuation will report no bifurcations)\n       rhsFn = (_y: Float64Array, _p: number, dydt: Float64Array) => {\n         for (let i = 0; i < nSpecies; i++) dydt[i] = 0;\n"}]},{"message":"The documentation claims the test verifies volume-based rate scaling with expected effective rate k/V = 0.5, but the test implementation (lines 155-166) does not validate this rate scaling. It only checks that C was produced and molecules are conserved. Either add assertions to verify the rate scaling or update the documentation to accurately reflect what the test validates.","fixFiles":[{"filePath":"tests/parity-compartment.spec.ts","diff":"diff --git a/tests/parity-compartment.spec.ts b/tests/parity-compartment.spec.ts\n--- a/tests/parity-compartment.spec.ts\n+++ b...

Copilot AI changed the title [WIP] Refactor bifurcation analysis to improve type safety Strengthen bifurcation handler typing and align compartment/URL codec tests with actual guarantees Apr 19, 2026
Copilot AI requested a review from akutuva21 April 19, 2026 16:11
Comment thread tests/parity-compartment.spec.ts Fixed
@akutuva21 akutuva21 marked this pull request as ready for review April 30, 2026 17:11
@akutuva21 akutuva21 merged commit dbf2621 into main Apr 30, 2026
8 checks passed
@akutuva21 akutuva21 deleted the copilot/refactor-bifurcation-analysis-type-safety branch April 30, 2026 17:12
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.

3 participants