Skip to content

Routing hygiene for agent harnesses, structured-output fidelity, OpenRouter hardening, shortfall semantic signals - #120

Open
veerareddyvishal144 wants to merge 5 commits into
mainfrom
bench/routing-and-provider-hardening
Open

veerareddyvishal144 wants to merge 5 commits into
mainfrom
bench/routing-and-provider-hardening

Conversation

@veerareddyvishal144

Copy link
Copy Markdown
Contributor

Summary

Found while benchmarking Lynkr tier routing against openrouter/auto on Terminal-Bench-core 0.1.1 (Terminus agent via LiteLLM, 80 tasks, ~1,200 calls per run). Every change is gated or scoped so Claude Code, Cursor and OpenCode keep their current behaviour unless a flag is set. Final numbers with these changes: 39/80 for $1.28 vs auto 44/80 for $4.72 on the same VM and evening.

Request / response fidelity

  • Forward the client's output_format / response_format to every provider (json_schema where the host supports it, json_object otherwise). Lynkr dropped both, so structured-output clients never got schema enforcement.
  • Never run the XML tool-call extractor on a JSON-object reply. It mangled command text containing <...> and flipped stop_reason to tool_use.
  • Structured-output requests bypass the response cache. Harnesses retry a failed parse with the identical conversation; a cache hit returned the same bad reply every time.
  • Remove server-side STANDARD_TOOLS injection from all provider invokers. A tool-less request stays tool-less. Injecting 12 Claude Code tool schemas turned chat / structured clients into tool-calling agents (+~2.5k tokens per call, write access to the host workspace). The OpenAI-compat router's IDE_SAFE_TOOLS injection is left untouched (tied to CLIENT_TOOL_MAPPINGS for Codex) — follow-up.
  • FMT_GUARD_ENABLED=false opt-out for the markdown format guard (its "use fenced code blocks" instruction contradicts raw-JSON clients).

Routing on the ask, not the wrapper

  • harness-envelope: recognise instruction-schema harness prompts (fixed preamble + JSON schema + Instruction: block) and expose the instruction as the ask for every turn. Complexity analyser, intent scorer, agentic detector, kNN query and window scorer use it. Later user turns are terminal output and previously drove force / risk / text scoring (62 mid-session COMPLEX→MEDIUM demotions in one run).
  • client-profiles: terminus profile (tool-less by design) with prompt-pattern detection; tool-less profiles excluded from side-request detection.
  • agentic-detector: drop solve from the autonomous regex — "solving command-line tasks" boilerplate pinned every request to REASONING.
  • FORCE_TIER_PATTERNS=false, RISK_TIER_ESCALATION=false opt-outs for the keyword escalators.
  • kNN: LYNKR_KNN_ENABLED=false skips the query and its embedding call; picks are validated against configured TIER_* models (the index remembered a decommissioned model and sent 81 calls to it).
  • embeddings: cap input at 5000 chars (nomic-embed-text's 2048-token context), surface Ollama's error body, retry once with halved text on input-too-long instead of degrading the provider for 60s.

Provider plumbing and verification

  • [ModelCheck] warning when the served model differs from the tier's requested model (live incident: TIER_*=zai:glm-5.3-flash silently served by ZAI_MODEL=glm-5.2 for two full runs).
  • Z.AI: unknown tier model ids pass through instead of falling back to the default model.
  • OpenRouter: per-model provider pin, per-model reasoning effort, strict json_schema for hosts that honour it, usage.include for provider-reported cached / reasoning tokens and cost, per-model upstream timeout with failover down the pinned list before tier-fallback climbs, per-model output cap, [ProviderCheck] warning when an unpinned host served. Replies converted in the invoker with cache_read mapped from prompt_tokens_details.cached_tokens.
  • Fireworks: per-model reasoning effort map, keep thinking enabled for models that reject thinking:disabled, response_format forwarding.
  • OpenAI-compatible: reasoning_effort knob and response_format forwarding.
  • Baidu Qianfan: clamp max_tokens to 12288 (hard provider limit).

Shortfall: semantic requirement + Jev telemetry (second commit)

  • Shortfall's requirement vector came from the structural complexity dimensions only and was nearly constant on task-description prompts (0.11–0.34 across 77 Terminal-Bench tasks), so cheapest-covering always picked the cheapest model. shortfall.liftRequirement now lifts each head with the anchor intent score (interpolated between tier profiles) and the Jev tier probabilities (probability-weighted profile); never lowers. Replay: requirement median 0.15 → 0.61, agreement with the anchor+Jev pick 51/61.
  • Jev verdicts were null on 100% of served telemetry rows while the judge re-tiered ~36% of fresh routes. Two gaps: the window wrapper stores the verdict as _jev (reader only knew jev/analysis.jev) and the forced-provider hop never carried it. jevFields reads _jev, api/router.js stamps req.body._jev, the reconstituted routingResult sets jev. Verified live: verdict, confidence, probabilities and judge model now land on every row.
  • New test/shortfall-semantic.test.js.

Test plan

  • npm run test:unit: 1562 pass / 6 fail on this branch and on main (the 6 are pre-existing, environment-dependent: Jev LRU, TencentDB launcher, downgrade gate, task-ledger).
  • Live: Terminus harness end-to-end through Lynkr on OpenRouter (Friendli glm-5.3-flash cheap tier, DeepSeek official strong tier): two full 80-task runs, 0 wrong-model warnings, 0 harness parse failures, failover drill (forced timeout → next host → tier climb) verified.
  • Requests that carry their own tools still forward them (probed: tool_use block returned).

New flags are documented in .env.example. Happy to split this into smaller PRs (tool-injection removal / structured output / harness awareness / OpenRouter hardening) if preferred.

🤖 Generated with Claude Code

veerareddyvishal144 and others added 4 commits October 3, 2026 21:43
…Router hardening

Found while benchmarking Lynkr tier routing against openrouter/auto on
Terminal-Bench-core 0.1.1 (Terminus agent via LiteLLM). Every change is
gated or scoped so existing clients (Claude Code, Cursor, OpenCode) keep
their current behaviour unless a flag is set.

Request/response fidelity
- Forward the client's output_format / response_format to every provider
  (json_schema where the host supports it, json_object otherwise). Lynkr
  dropped both, so structured-output clients never got schema enforcement.
- Never run the XML tool-call extractor on a reply that is a JSON object;
  it mangled command text containing angle brackets and flipped
  stop_reason to tool_use. (databricks.js converter + orchestrator)
- Structured-output requests bypass the response cache: agent harnesses
  retry a failed parse with the identical conversation, and a cache hit
  returned the same bad reply every time (LYNKR_CACHE_BYPASS_STRUCTURED).
- Remove server-side STANDARD_TOOLS injection from all provider invokers.
  A tool-less request stays tool-less; injecting 12 Claude Code tool
  schemas turned chat/structured clients into tool-calling agents
  (+~2.5k tokens/call, write access to the host workspace). The OpenAI-
  compat router's IDE_SAFE_TOOLS injection is left untouched (tied to
  CLIENT_TOOL_MAPPINGS for Codex) — follow-up.
- FMT_GUARD_ENABLED=false opt-out for the markdown format guard, whose
  "use fenced code blocks" instruction contradicts raw-JSON clients.

Routing on the ask, not the wrapper
- harness-envelope: recognise instruction-schema harness prompts
  (Terminus-style preamble + JSON schema + "Instruction:" block) and
  expose the instruction as the ask for every turn of the session. The
  complexity analyser, intent scorer, agentic detector, kNN query and
  the window scorer all use it; later user turns are terminal output and
  previously drove force/risk/text scoring (v12: 62 mid-session
  COMPLEX->MEDIUM demotions from scoring stdout).
- client-profiles: 'terminus' profile (tool-less by design) + prompt-
  pattern detection on the first user message; tool-less profiles are
  excluded from side-request detection.
- agentic-detector: drop "solve" from the autonomous regex — harness
  boilerplate ("solving command-line tasks") pinned every request to
  REASONING.
- FORCE_TIER_PATTERNS=false and RISK_TIER_ESCALATION=false opt-outs for
  the keyword escalators (tasks about security/verification are not
  themselves risky).
- kNN: LYNKR_KNN_ENABLED=false skips the query and its embedding call;
  kNN picks are validated against the configured TIER_* models (the
  index remembered a decommissioned model and sent 81 calls to it).
- embeddings: cap input at 5000 chars (nomic-embed-text 2048-token
  context), surface Ollama's error body, retry once with halved text on
  an input-too-long rejection instead of degrading the provider for 60s.

Provider plumbing and verification
- invokeProvider: [ModelCheck] warning when the served model differs
  from the tier's requested model (live incident: TIER zai:glm-5.3-flash
  silently served by ZAI_MODEL=glm-5.2 for two full runs).
- Z.AI: unknown tier model ids pass through instead of falling back to
  the configured default model.
- OpenRouter: per-model provider pin (OPENROUTER_PROVIDER_ORDER[_MAP]),
  per-model reasoning effort, strict json_schema for hosts that honour
  it (OPENROUTER_SCHEMA_PROVIDERS), usage.include for provider-reported
  cached/reasoning tokens and cost, per-model upstream timeout
  (OPENROUTER_TIMEOUT_MS[_MAP]) with failover down the pinned list before
  tier-fallback climbs, per-model output cap (OPENROUTER_MAX_TOKENS_MAP),
  [ProviderCheck] warning when an unpinned host served. Replies are
  converted in the invoker with the hardened converter (cache_read
  mapped from prompt_tokens_details.cached_tokens); the orchestrator
  passes type:"message" through.
- Fireworks: per-model reasoning effort map, keep thinking enabled for
  models that reject thinking:disabled, response_format forwarding.
- OpenAI-compatible: reasoning_effort knob and response_format forwarding.
- Baidu Qianfan: clamp max_tokens to 12288 (hard provider limit).

Tests: npm run test:unit — 1562 pass / 6 fail on both this branch and
main (the 6 are pre-existing, environment-dependent). New flags
documented in .env.example.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ord Jev on the served path

Shortfall built its requirement vector from the STRUCTURAL complexity
dimensions only (message length, tool count, code blocks, turns). On
task-description prompts that vector is nearly constant — measured
0.11–0.34 (median 0.15) across 77 Terminal-Bench tasks, preamble or
not — so cheapest-covering always chose the cheapest model and the Jev
floor vetoed it every time. The main routing path already computes two
semantic difficulty signals shortfall never saw.

- shortfall.liftRequirement(req, { anchorScore, jev }): per head, take
  the max of the structural requirement, the anchor-score requirement
  (interpolated between adjacent tier profiles at band midpoints 10/35/
  63/88) and the Jev requirement (tier-probability-weighted tier
  profile). Never lowers a head; null/invalid signals are ignored.
  Replay on the 77 tasks: requirement median 0.15 → 0.61, p90 0.30 →
  0.81; shortfall now agrees with the anchor+Jev pick on 51/61 (the
  remaining 10 are tau permissiveness, not signal).
- routing/index.js feeds analysis.anchorScore / analysis.jev into the
  lift and records structuralReq + applied lifts in the shadow log and
  shortfallInfo.
- Jev verdicts were null on 100% of served telemetry rows while the
  judge was re-tiering ~36% of fresh routes (replay: 11 raised, 17
  lowered of 77). Two gaps: the window-scoring wrapper stores the
  verdict as `_jev` (telemetry.jevFields only read `jev`/`analysis.jev`),
  and the forced-provider hop (api/router.js → invokeModel) never
  carried it, so the reconstituted routingResult had no verdict at all.
  jevFields now also reads `_jev`; router.js stamps `req.body._jev`;
  the forced-path routingResult sets `jev: body._jev`.
- test/shortfall-semantic.test.js (registered in test:unit).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… measured solo runs

One command: (1) optionally benchmark each --model solo through a throwaway
Lynkr on a spare port (all tiers pinned, own .env/telemetry; needs --run),
(2) replay every task's first prompt through the router for its lifted
requirement vector (cached), (3) fit per-model/head capability as the
highest difficulty level still passed at >= --floor and >= --rel x the
best model, tau from the collapse gap, (4) hold-out check of shortfall vs
best-model-everywhere on accuracy and cost, report + proposed overrides,
--apply to merge into config/model-capabilities.json (backup first).
Reuses existing Terminal-Bench run dirs via --model spec=dir.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…d execSync import

CI's blocking lint step (eslint --max-warnings 0) failed on two unused
bindings: anthropicOutputFormat was added with the structured-output
forwarding but the Z.AI path ended up using the OpenAI-compat
response_format instead; execSync in cursor-utils.js was unused on main
already (main's last two CI runs failed on it).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

🔍 OpenCodeReview found 23 issue(s) in this PR.

  • ✅ Successfully posted inline: 23 comment(s)

Comment on lines +415 to +416
fs.copyFileSync(CONFIG_PATH, CONFIG_PATH + `.bak-${Date.now()}`);
cfg.modelOverrides = { ...(cfg.modelOverrides || {}), ...overrides };

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.

security · medium
The backup file write is not validated before overwriting the config. If copyFileSync fails (e.g., permission issues), the config could be partially overwritten or lost. Add a check after copyFileSync to ensure the backup exists before proceeding with the rewrite.

Comment thread src/cache/embeddings.js
// absorbs the blip; a provider that is actually down fails twice and
// degrades exactly as before.
const TRANSIENT_RETRY_DELAY_MS = 1500;
const _INPUT_LENGTH_RE = /context length|input length|too long|exceeds? (the )?(maximum|context)|maximum context/i;

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.

bug · low
The _INPUT_LENGTH_RE regex at line 184 is overly broad and could match false positives. The pattern context length|input length|too long|exceeds? (the )?(maximum|context)|maximum context would incorrectly match valid messages containing these words. For example, "too long" appears in many contexts unrelated to errors. Consider making the regex more specific by anchoring to error contexts (e.g., case sensitivity, surrounding keywords like "error", "rejected", or requiring specific error message patterns from known providers).

Suggestion:

Suggested change
const _INPUT_LENGTH_RE = /context length|input length|too long|exceeds? (the )?(maximum|context)|maximum context/i;
const _INPUT_LENGTH_RE = /\b(context|input)\s+length\s+\b(exceeds?|too long)\b|\bmaximum\s+(context|length)\b|\binput\s+too\s+long\b/i;

Comment thread src/clients/databricks.js
// fall back to the configured default. Live incident: TIER_*=zai:glm-5.3-flash
// was silently served by ZAI_MODEL=glm-5.2 for two full benchmark runs
// (response body "model":"glm-5.2" while Lynkr reported glm-5.3-flash).
let mappedModel = modelMap[requestedModel] || (body._tierModel ? requestedModel : null) || config.zai.model || "glm-4.7";

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.

bug · medium
At line 2266, body._tierModel is accessed without null checking. When body._tierModel is falsy, the expression (body._tierModel ? requestedModel : null) correctly returns null, but if _tierModel is an empty string or 0, it will use requestedModel (the original value). This could lead to unexpected behavior. Better to explicitly check typeof body._tierModel === 'string' && body._tierModel to ensure it's a valid non-empty string.

Comment thread src/orchestrator/index.js
Comment on lines +2318 to +2319
const _bt = block.text.trim();
if (_bt.startsWith("{") && _bt.endsWith("}")) continue;

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.

bug · medium
In src/orchestrator/index.js, the JSON detection check startsWith("{") && endsWith("}") is fragile and can be bypassed by strings containing JSON anywhere within them. For example, a string like "Here is the JSON: {"key": "value"}" would incorrectly skip extraction. A more robust approach would be to try parsing with JSON.parse() in a try-catch block, or check for a more specific pattern.

Suggestion:

Suggested change
const _bt = block.text.trim();
if (_bt.startsWith("{") && _bt.endsWith("}")) continue;
const _bt = block.text.trim();
try {
JSON.parse(_bt); continue; // Valid JSON object
} catch {}
// Only try to parse first/last char if trim didn't change the string
if (_bt === block.text && _bt.startsWith("{") && _bt.endsWith("}")) continue;

Comment thread src/orchestrator/index.js
Comment on lines +2346 to +2347
if (toolCalls.length === 0 && typeof message.content === "string" && message.content.trim()
&& !(message.content.trim().startsWith("{") && message.content.trim().endsWith("}"))) {

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.

maintainability · low
In src/orchestrator/index.js, there's duplicated logic for JSON detection on two separate code paths (lines 2318-2319 and 2346-2347). This could be extracted into a helper function like isJsonReply(content) to avoid duplication.

Suggestion:

Suggested change
if (toolCalls.length === 0 && typeof message.content === "string" && message.content.trim()
&& !(message.content.trim().startsWith("{") && message.content.trim().endsWith("}"))) {
const isJsonReply = (text) => {
const trimmed = text.trim();
if (trimmed === text && trimmed.startsWith("{") && trimmed.endsWith("}")) return true;
try { JSON.parse(trimmed); return true; } catch {}
return false;
};
// Then use: !isJsonReply(message.content)

Comment on lines +997 to 998
if (process.env.FORCE_TIER_PATTERNS === "false") return false;
const content = extractContent(payload);

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.

bug · low
In src/routing/complexity-analyzer.js, both shouldForceCloud and shouldForceReasoning extract content using extractContent(payload) which could return '' (empty string) if payload has no messages. The regex patterns might match unexpected empty content. Consider adding an explicit check for empty content before pattern matching.

Suggestion:

Suggested change
if (process.env.FORCE_TIER_PATTERNS === "false") return false;
const content = extractContent(payload);
if (process.env.FORCE_TIER_PATTERNS === "false") return false;
const content = extractContent(payload);
if (!content || !content.trim()) return false;

Comment thread src/routing/index.js
Comment on lines +1774 to +1776
if (knnResult && process.env.LYNKR_KNN_ENABLED === 'false') {
knnResult = null;
}

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.

maintainability · low
In src/routing/index.js, the check if (knnResult && process.env.LYNKR_KNN_ENABLED === 'false') is redundant when the outer code already checks config.routing?.knnEnabled !== false && process.env.LYNKR_KNN_ENABLED !== 'false'. The inner check will never be true unless LYNKR_KNN_ENABLED changes between checks (which shouldn't happen). This nested check is defensive but unnecessary.

Suggestion:

Suggested change
if (knnResult && process.env.LYNKR_KNN_ENABLED === 'false') {
knnResult = null;
}
// This check is redundant given the outer condition already checks LYNKR_KNN_ENABLED
// Consider removing or adding a comment explaining why it's needed

Comment thread src/routing/index.js
Comment on lines +1779 to +1781
const _sel = getModelTierSelector();
const _allowed = new Set();
for (const _t of TIER_ORDER) for (const _m of (_sel.getModelsForTier(_t) || [])) _allowed.add(`${_m.provider}:${_m.model}`);

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.

maintainability · medium
In src/routing/index.js, the kNN tier validation code has duplicated logic in two try-catch blocks (one for success, one for error handling). The same model set construction code appears twice. This could be extracted into a helper function to avoid duplication.

Suggestion:

Suggested change
const _sel = getModelTierSelector();
const _allowed = new Set();
for (const _t of TIER_ORDER) for (const _m of (_sel.getModelsForTier(_t) || [])) _allowed.add(`${_m.provider}:${_m.model}`);
const getModelSetFromTiers = () => {
const _sel = getModelTierSelector();
const _allowed = new Set();
for (const _t of TIER_ORDER) for (const _m of (_sel.getModelsForTier(_t) || [])) _allowed.add(`${_m.provider}:${_m.model}`);
return _allowed;
};
// Then use: const _allowed = getModelSetFromTiers();

Comment thread src/routing/shortfall.js
* Lift a structural requirement vector with the semantic signals.
* @returns {{ req: object, lift: { anchor: object|null, jev: object|null, applied: string[] } }}
*/
function liftRequirement(req, { anchorScore = null, jev = null } = {}, tierProfiles = null) {

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.

bug · low
In src/routing/shortfall.js, the liftRequirement function uses default parameters (anchorScore = null, jev = null) which is good. However, it doesn't validate that the input req object actually has the expected properties (HEADS), which could cause issues if an unexpected object is passed.

Suggestion:

Suggested change
function liftRequirement(req, { anchorScore = null, jev = null } = {}, tierProfiles = null) {
function liftRequirement(req, { anchorScore = null, jev = null } = {}, tierProfiles = null) {
if (!req || typeof req !== 'object') return { req: {}, lift: { anchor: null, jev: null, applied: [] } };

Comment thread src/routing/shortfall.js
Comment on lines +366 to +368
for (const h of HEADS) {
if (a && a[h] > out[h]) { out[h] = a[h]; if (!applied.includes('anchor')) applied.push('anchor'); }
if (j && j[h] > out[h]) { out[h] = j[h]; if (!applied.includes('jev')) applied.push('jev'); }

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.

performance · low
The liftRequirement function uses Array.includes() to track which lifts were applied ('anchor', 'jev'). This is inefficient and could be simplified by just counting the number of successful lifts instead of maintaining an array.

Suggestion:

Suggested change
for (const h of HEADS) {
if (a && a[h] > out[h]) { out[h] = a[h]; if (!applied.includes('anchor')) applied.push('anchor'); }
if (j && j[h] > out[h]) { out[h] = j[h]; if (!applied.includes('jev')) applied.push('jev'); }
const appliedSet = new Set();
for (const h of HEADS) {
if (a && a[h] > out[h]) { out[h] = a[h]; appliedSet.add('anchor'); }
if (j && j[h] > out[h]) { out[h] = j[h]; appliedSet.add('jev'); }
out[h] = Math.round(Math.max(0, Math.min(1, out[h])) * 1000) / 1000;
}
const applied = [...appliedSet];

Removes the "2026-xx-xx local patch" / incident-narration comments added
alongside the changes; the PR description carries the rationale. Code is
unchanged (eslint clean, unit suite identical: 1569 pass / same 4
pre-existing failures).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Comment thread package.json
"lint": "eslint src index.js",
"test": "npm run test:unit && npm run test:performance",
"test:unit": "LYNKR_KNN_DIR=/tmp/lynkr-test-knn DATABRICKS_API_KEY=test-key DATABRICKS_API_BASE=http://test.com LOG_FILE_ENABLED=false node --test test/routing.test.js test/hybrid-routing-integration.test.js test/retry-logic.test.js test/sse-transformer.test.js test/passthrough-stream.test.js test/passthrough-mode.test.js test/openrouter-error-resilience.test.js test/format-conversion.test.js test/azure-openai-config.test.js test/azure-openai-format-conversion.test.js test/azure-openai-routing.test.js test/azure-openai-streaming.test.js test/azure-openai-error-resilience.test.js test/azure-openai-integration.test.js test/openai-integration.test.js test/atlas-integration.test.js test/toon-compression.test.js test/gcf-compression.test.js test/llamacpp-integration.test.js test/resilience.test.js test/telemetry-routing.test.js test/memory/store.test.js test/memory/surprise.test.js test/memory/extractor.test.js test/memory/search.test.js test/memory/retriever.test.js test/memory/distiller.test.js test/memory/distiller-freeze.test.js test/memory/wiki.test.js test/memory/skills-cache.test.js test/memory/tencentdb-launcher.test.js test/distill.test.js test/large-payload.test.js test/prompt-cache-injection.test.js test/risk-analyzer.test.js test/interaction-block.test.js test/preflight.test.js test/token-reduction.test.js test/session-affinity.test.js test/cache-state.test.js test/cache-switch-cost.test.js test/lens-recommendations.test.js test/model-registry-cost.test.js test/output-format-guard.test.js test/tier-fallback.test.js test/wrap.test.js test/init.test.js test/tool-call-response-metadata.test.js test/degradation.test.js test/routing-telemetry-columns.test.js test/sticky-routing.test.js test/knn-ambiguous-escalate.test.js test/deescalator.test.js test/client-profiles.test.js test/strip-internal-fields.test.js test/complexity-tool-subtraction.test.js test/bandit.test.js test/routing-propensity.test.js test/reward-pipeline.test.js test/knn-cold-start.test.js test/calibration.test.js test/feedback-loop.test.js test/session-fingerprint.test.js test/side-request-guards.test.js test/verifier.test.js test/intent-score.test.js test/difficulty-classifier.test.js test/classifier-setup.test.js test/usage-stats.test.js test/loop-guard.test.js test/moonshot-model-mapping.test.js test/baidu-model-mapping.test.js test/tenant-policy-ingress-parity.test.js test/decide.test.js test/embeddings-degradation.test.js test/health-probe.test.js test/stuck-detector.test.js test/onnx-embedder.test.js test/ope.test.js test/hierarchical-budget.test.js test/token-rate-limit.test.js test/otel-export.test.js test/mcp-broker.test.js test/compression-budget.test.js test/gpt-utils.test.js test/dedup-observe-only.test.js test/context-window-header.test.js test/token-budget-auto.test.js test/opencode-setup.test.js test/auth-mode-first-party.test.js test/harness-envelope.test.js test/task-ledger.test.js test/jev-router.test.js test/jev-routing.test.js test/force-patterns.test.js test/upstream-fidelity.test.js test/tool-schema-compression.test.js test/passthrough-route.test.js test/quota-ledger.test.js",
"test:unit": "LYNKR_KNN_DIR=/tmp/lynkr-test-knn DATABRICKS_API_KEY=test-key DATABRICKS_API_BASE=http://test.com LOG_FILE_ENABLED=false node --test test/routing.test.js test/hybrid-routing-integration.test.js test/retry-logic.test.js test/sse-transformer.test.js test/passthrough-stream.test.js test/passthrough-mode.test.js test/openrouter-error-resilience.test.js test/format-conversion.test.js test/azure-openai-config.test.js test/azure-openai-format-conversion.test.js test/azure-openai-routing.test.js test/azure-openai-streaming.test.js test/azure-openai-error-resilience.test.js test/azure-openai-integration.test.js test/openai-integration.test.js test/atlas-integration.test.js test/toon-compression.test.js test/gcf-compression.test.js test/llamacpp-integration.test.js test/resilience.test.js test/telemetry-routing.test.js test/memory/store.test.js test/memory/surprise.test.js test/memory/extractor.test.js test/memory/search.test.js test/memory/retriever.test.js test/memory/distiller.test.js test/memory/distiller-freeze.test.js test/memory/wiki.test.js test/memory/skills-cache.test.js test/memory/tencentdb-launcher.test.js test/distill.test.js test/large-payload.test.js test/prompt-cache-injection.test.js test/risk-analyzer.test.js test/interaction-block.test.js test/preflight.test.js test/token-reduction.test.js test/session-affinity.test.js test/cache-state.test.js test/cache-switch-cost.test.js test/lens-recommendations.test.js test/model-registry-cost.test.js test/output-format-guard.test.js test/tier-fallback.test.js test/wrap.test.js test/init.test.js test/tool-call-response-metadata.test.js test/degradation.test.js test/routing-telemetry-columns.test.js test/sticky-routing.test.js test/knn-ambiguous-escalate.test.js test/deescalator.test.js test/client-profiles.test.js test/strip-internal-fields.test.js test/complexity-tool-subtraction.test.js test/bandit.test.js test/routing-propensity.test.js test/reward-pipeline.test.js test/knn-cold-start.test.js test/calibration.test.js test/feedback-loop.test.js test/session-fingerprint.test.js test/side-request-guards.test.js test/verifier.test.js test/intent-score.test.js test/difficulty-classifier.test.js test/classifier-setup.test.js test/usage-stats.test.js test/loop-guard.test.js test/moonshot-model-mapping.test.js test/baidu-model-mapping.test.js test/tenant-policy-ingress-parity.test.js test/decide.test.js test/embeddings-degradation.test.js test/health-probe.test.js test/stuck-detector.test.js test/onnx-embedder.test.js test/ope.test.js test/hierarchical-budget.test.js test/token-rate-limit.test.js test/otel-export.test.js test/mcp-broker.test.js test/compression-budget.test.js test/gpt-utils.test.js test/dedup-observe-only.test.js test/context-window-header.test.js test/token-budget-auto.test.js test/opencode-setup.test.js test/auth-mode-first-party.test.js test/harness-envelope.test.js test/task-ledger.test.js test/jev-router.test.js test/jev-routing.test.js test/force-patterns.test.js test/upstream-fidelity.test.js test/tool-schema-compression.test.js test/passthrough-route.test.js test/quota-ledger.test.js test/shortfall-semantic.test.js",

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.

other · low
package.json update: Added test/shortfall-semantic.test.js to test:unit. No dependency conflicts or duplicate declarations found. All tools referenced in scripts (eslint, pino-pretty) are declared in devDependencies.

Comment on lines +75 to +76
const v = next(); const eq = v.indexOf('=');
out.models.push(eq > 0 ? { spec: v.slice(0, eq), runDir: expand(v.slice(eq + 1)) } : { spec: v, runDir: null });

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.

security · low
Unsanitized file paths: The script reads user-supplied paths (--model=run-dir, --out) and writes to them without validation.

Suggestion:

Suggested change
const v = next(); const eq = v.indexOf('=');
out.models.push(eq > 0 ? { spec: v.slice(0, eq), runDir: expand(v.slice(eq + 1)) } : { spec: v, runDir: null });
// Validate runDir exists and is accessible before using
if (m.runDir && !fs.existsSync(m.runDir)) die(`--model run-dir does not exist: ${m.runDir}`);

else if (a === '--apply') out.apply = true;
else if (a === '--dry-run') out.dryRun = true;
else if (a === '--help' || a === '-h') { console.log(fs.readFileSync(__filename, 'utf8').split('*/')[0].replace(/^\/\*\*?\s?/, '').replace(/^ \* ?/gm, '')); process.exit(0); }
else die(`unknown arg ${a}`);

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.

bug · low
Inconsistent error messages: --help prints usage before exit, but validation errors via die() don't. Users encountering errors may not know valid arguments.

Suggestion:

Suggested change
else die(`unknown arg ${a}`);
else { console.error(`unknown arg: ${a}`); console.error('Run with --help for usage'); die(''); }

fs.mkdirSync(home, { recursive: true }); fs.mkdirSync(outDir, { recursive: true });
// Throwaway Lynkr: copy the operator .env, pin all tiers to this model,
// own port, own telemetry dir (dotenv + telemetry both key off cwd).
const srcEnv = fs.existsSync(path.join(process.cwd(), '.env')) ? path.join(process.cwd(), '.env') : path.join(os.homedir(), '.env');

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.

maintainability · low
Hardcoded .env paths: The script assumes .env is in process.cwd() or os.homedir(). This may fail in containerized or different working directory contexts.

Suggestion:

Suggested change
const srcEnv = fs.existsSync(path.join(process.cwd(), '.env')) ? path.join(process.cwd(), '.env') : path.join(os.homedir(), '.env');
// Add --env-path flag or look for .env relative to ROOT, then cwd, then homedir
const envPaths = [path.join(ROOT, '.env'), path.join(process.cwd(), '.env'), path.join(os.homedir(), '.env')];

fs.mkdirSync(home, { recursive: true }); fs.mkdirSync(outDir, { recursive: true });
// Throwaway Lynkr: copy the operator .env, pin all tiers to this model,
// own port, own telemetry dir (dotenv + telemetry both key off cwd).
const srcEnv = fs.existsSync(path.join(process.cwd(), '.env')) ? path.join(process.cwd(), '.env') : path.join(os.homedir(), '.env');

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.

maintainability · low
Repeated string pattern: fs.existsSync(path.join(process.cwd(), '.env')) ? path.join(process.cwd(), '.env') : path.join(os.homedir(), '.env') appears twice and should be extracted.

Suggestion:

Suggested change
const srcEnv = fs.existsSync(path.join(process.cwd(), '.env')) ? path.join(process.cwd(), '.env') : path.join(os.homedir(), '.env');
function findEnvPath() {
const cwd = path.join(process.cwd(), '.env');
if (fs.existsSync(cwd)) return cwd;
return path.join(os.homedir(), '.env');
}

Comment on lines +41 to +45
detect: {
headerPatterns: [],
promptPatterns: [/You are an AI assistant tasked with solving command-line tasks/i],
minToolFingerprintMatch: 1,
},

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.

maintainability · low
The terminus profile's prompt pattern matches the HARNESS_PREAMBLE_RES pattern in harness-envelope.js. This creates tight coupling between client detection and harness parsing logic. Consider unifying these patterns or moving to a shared configuration file.

return '';
}
{
const { harnessAskFromPayload } = require('./harness-envelope');

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.

other · low
Same dynamic require pattern here - could cause circular dependency issues if the module loading order changes.

Comment on lines +51 to +54
const HARNESS_PREAMBLE_RES = [
/You are an AI assistant tasked with solving command-line tasks/i,
/"title":\s*"CommandBatchResponse"/,
];

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.

bug · high
The harness prompt detection pattern /You are an AI assistant tasked with solving command-line tasks/i is very broad and may match non-harness prompts from various providers. Consider making it more specific to your actual harness system.

Suggestion:

Suggested change
const HARNESS_PREAMBLE_RES = [
/You are an AI assistant tasked with solving command-line tasks/i,
/"title":\s*"CommandBatchResponse"/,
];
const HARNESS_PREAMBLE_RES = [
// Match more specific harness identifiers to avoid false positives
/You are an AI assistant.*?command-line tasks.*?harness/i,
/"title":\s*"CommandBatchResponse"/,
];

Comment thread src/routing/index.js
Comment on lines +1762 to +1763
const _allowed = new Set();
for (const _t of TIER_ORDER) for (const _m of (_sel.getModelsForTier(_t) || [])) _allowed.add(`${_m.provider}:${_m.model}`);

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.

performance · low
The kNN tier validation loop creates a Set and iterates all tiers every request. If TIER_ORDER has many tiers/models, this could add latency to every routed request.

const msgs = payload?.messages;
if (!Array.isArray(msgs)) return { text: null, index: -1 };
{
const { harnessAskFromPayload } = require('./harness-envelope');

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.

other · low
Same dynamic require pattern here - could cause circular dependency issues if the module loading order changes.

This branch has not been deployed

No deployments
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