Skip to content

feat: add requestTransform for deterministic matching and recording#63

Open
iskhakovt wants to merge 2 commits intoCopilotKit:mainfrom
iskhakovt:feat/request-transform
Open

feat: add requestTransform for deterministic matching and recording#63
iskhakovt wants to merge 2 commits intoCopilotKit:mainfrom
iskhakovt:feat/request-transform

Conversation

@iskhakovt
Copy link
Copy Markdown
Contributor

@iskhakovt iskhakovt commented Mar 30, 2026

Problem

LLM prompts often contain dynamic data (timestamps, UUIDs, session IDs)
injected by upstream services. When recording fixtures, the match key
includes this dynamic data. On replay, the same prompt with different
timestamps doesn't match the stored fixture.

Example: Hindsight memory server injects Event Date: 2026-03-30T13:19:38
into extraction prompts. Each test run has a different timestamp, so
recorded fixtures never match on replay.

Solution

Add requestTransform to MockServerOptions — a function that normalizes
requests before both matching and recording:

const mock = new LLMock({
  requestTransform: (req) => ({
    ...req,
    messages: req.messages.map(m => ({
      ...m,
      content: typeof m.content === "string"
        ? m.content.replace(/\d{4}-\d{2}-\d{2}T[\d:.+Z]+/g, "")
        : m.content,
    })),
    embeddingInput: req.embeddingInput?.split(" | ")[0],
  }),
});

Recording: saves the transformed match key — no timestamps in fixture.
Matching: transforms incoming request before comparison — same clean key.

When requestTransform is set, string matching switches from includes
(substring) to === (exact equality). This prevents false positive matches
from shortened keys accidentally matching unrelated prompts. Without a
transform, existing includes behavior is preserved (backward compatible).

Follows the Polly.js pattern of
composable request normalizers for deterministic snapshot matching.

Changes

  • types.ts: Add requestTransform to MockServerOptions and HandlerDefaults
  • router.ts: Optional 4th param on matchFixture, exact match when transform set
  • server.ts: Thread transform into defaults
  • recorder.ts: Apply transform before match key extraction
  • All handlers: Pass defaults.requestTransform as 4th arg to matchFixture
  • docs/record-replay.html: Document requestTransform feature
  • 8 new router tests for transform behavior, exact matching, backward compat

@iskhakovt iskhakovt force-pushed the feat/request-transform branch from bac282e to 0207739 Compare March 30, 2026 18:52
…ording

Optional requestTransform on MockServerOptions normalizes requests before
fixture matching. When set, string comparisons use exact equality (===)
instead of includes() for deterministic recorded-fixture replay.

- matchFixture gets optional 4th parameter, threaded from all handlers
- Recorder applies transform before building fixture match keys
- 8 new tests cover transform behavior, backward compat, and predicate passthrough

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@iskhakovt iskhakovt force-pushed the feat/request-transform branch from 0207739 to 958add3 Compare March 30, 2026 19:43
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@iskhakovt iskhakovt marked this pull request as ready for review March 30, 2026 20:34
Copy link
Copy Markdown

@claude claude bot left a comment

Choose a reason for hiding this comment

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

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@iskhakovt iskhakovt force-pushed the feat/request-transform branch 2 times, most recently from 9694329 to 75d0dfa Compare March 31, 2026 02:24
@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new bot commented Mar 31, 2026

Open in StackBlitz

npm i https://pkg.pr.new/CopilotKit/llmock/@copilotkit/llmock@63

commit: 75d0dfa

Copy link
Copy Markdown
Contributor

@jpr5 jpr5 left a comment

Choose a reason for hiding this comment

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

Code Review — 7-Agent Standard CR

335 lines across 17 files reviewed by 7 specialized agents.

The feature design is sound and well-motivated — requestTransform solves a real problem with dynamic data in recorded fixtures. The implementation is mechanically correct across all 17 files, with consistent threading of the new parameter through every handler. Three items need addressing before merge.


Bugs

1. Docs claim "RegExp and predicate matching are unaffected" — RegExp IS affected

docs/record-replay.html states:

RegExp and predicate matching are unaffected

But the code in router.ts applies effectiveReq (the transformed request) to all matching criteria including regex — only predicates receive the original req. Your own test at line 125 ("regexp does not match when transform changes the text") proves this by asserting that regex fails when the transform changes the content.

Fix: Change to something like:

Only predicate matching is unaffected — predicates always receive the original (untransformed) request. All other match criteria (including RegExp) operate on the transformed request.

Also update the JSDoc on requestTransform in types.ts — it mentions matching but omits the effect on recording and the includes()=== behavioral switch.

2. Transform that throws crashes the handler with opaque "Internal error"

requestTransform is user-supplied code called unprotected in matchFixture():

const effectiveReq = requestTransform ? requestTransform(req) : req;

If the transform throws (TypeError, user logic error, etc.), the exception propagates up through every handler into the HTTP response path. Users get a generic 500 "Internal error" with zero indication their transform was the problem.

In recorder.ts, it's worse — the upstream response has already been fetched but is lost when the transform throws during fixture-key building.

In WebSocket handlers, the matchFixture call is outside the JSON parse try/catch, so a throwing transform crashes the message handler.

Fix: Wrap the transform invocation in try/catch. Log the actual error with context identifying the transform as the source. Return null from matchFixture (no match) on failure, or create a small wrapper:

let effectiveReq: ChatCompletionRequest;
try {
  effectiveReq = requestTransform ? requestTransform(req) : req;
} catch (err) {
  // need logger access — either pass it in or wrap at call sites
  return null;
}

Since matchFixture doesn't have logger access, the wrapping may need to happen at call sites. Either approach works.

3. Transform that drops messages causes TypeError

If a user's transform returns an object without messages (e.g. { model: req.model }), getLastMessageByRole(effectiveReq.messages, "user") throws TypeError: Cannot read properties of undefined (reading 'length'). TypeScript interfaces provide no runtime enforcement.

Fix: Either validate the transform output at the top of matchFixture, or add a guard before getLastMessageByRole:

if (match.userMessage !== undefined) {
  if (!effectiveReq.messages?.length) continue;
  const msg = getLastMessageByRole(effectiveReq.messages, "user");
  // ...
}

Missing Test

4. No test for throwing transform

User-supplied code in a hot path with no error handling deserves a test that documents the contract — whether matchFixture propagates the exception or handles it gracefully. Either behavior is fine, but it should be tested and intentional, not accidental.


Design Discussion (non-blocking)

5. Identity transform silently changes matching semantics

The mere presence of ANY transform (even (r) => r) switches string matching from includes() to ===. A user adding a no-op transform expecting no behavioral change will find previously-matching fixtures stop matching. Worth considering whether exact-match should be a separate exactMatch?: boolean option rather than coupled to transform presence.

6. Transform can mutate original request

If a user writes req.messages = req.messages.filter(...) (mutating in place), effectiveReq === req and the "predicates receive original" contract is silently violated. The docs example uses spread correctly, but nothing enforces immutability. Consider documenting this prominently or using structuredClone(req) before passing to the transform.

7. Type duplication across 8+ locations

The 3 WebSocket handler files inline their own defaults type (6 copies) instead of referencing HandlerDefaults. recorder.ts also has its own inline type. Consider extracting:

export type WebSocketHandlerDefaults = HandlerDefaults & { model: string };
export type RequestTransform = (req: ChatCompletionRequest) => ChatCompletionRequest;

This is a pre-existing pattern the PR inherits, not something it introduced — but since you're touching all these signatures anyway, it's a good time to clean it up.


What's Good

  • Clean, consistent threading of the new parameter through all 15 matchFixture call sites
  • Predicate isolation (receiving original req) is a well-thought-out design choice
  • Dual application in recorder (normalize both at match time and record time) ensures round-trip consistency
  • 8 router tests cover the core behavioral contract well: exact match, regex, embedding, backward compat, predicate isolation
  • Documentation section with realistic code example

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.

2 participants