Skip to content

fix(openai): use fs paths for embedded fixtures - #2484

Merged
looplj merged 1 commit into
looplj:unstablefrom
remote-controlled-man:fix/windows-embedded-fixtures
Sep 16, 2026
Merged

looplj merged 1 commit into
looplj:unstablefrom
remote-controlled-man:fix/windows-embedded-fixtures

Conversation

@remote-controlled-man

@remote-controlled-man remote-controlled-man commented Sep 15, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Use path.Join instead of filepath.Join for fixtures read from embed.FS.
  • Make both non-streaming and streaming fake-executor fixture loads portable on Windows.
  • Leave fixture contents and response behavior unchanged.

Problem

Names passed to embed.FS.ReadFile follow the io/fs convention and always use forward slashes. filepath.Join uses the host operating system's separator, so on Windows it produces names such as testdata\openai-stop.response.json.

Those names do not identify the slash-separated fixtures in embed.FS, causing both TestFakeExecutor_Do and TestFakeExecutor_DoStream to fail with file does not exist before exercising the transformer behavior.

Using path.Join preserves slash-separated embedded-FS names on every operating system.

Related work

The same fixture-path correction previously appeared inside broader changes in #1940, #2312, and #2397, but it did not land in unstable. This PR isolates only the portable fixture lookup change; the merged #2399 does not modify fake.go.

Validation

  • Before the change, go test ./transformer/openai -run '^TestFakeExecutor_(Do|DoStream)$' -count=1 -v failed both tests with testdata\...: file does not exist.
  • The targeted tests passed with -count=5 on the reviewed unstable@ad5b5eec base (10 executions total).
  • go test ./transformer/openai -count=1 passed on the reviewed unstable@ad5b5eec base.
  • gofmt -d llm/transformer/openai/fake.go produced no diff.
  • git diff --check passed.
  • go test ./... -count=1 completed across the llm module. Every package passed except transformer/openai/responses, whose sole failing Windows WebSocket assertion also fails unchanged on unstable@ad5b5eec; it is unrelated to this one-file fixture-path change.

Risk

The change is limited to embedded test-fixture lookup. It does not alter fixture data, request transformation, or response parsing.

Summary by CodeRabbit

  • Tests
    • Updated test response file path handling without changing observable responses or error behavior.

Copilot AI lite review requested due to automatic review settings September 15, 2026 13:41
@coderabbitai

coderabbitai Bot commented Sep 15, 2026 •

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: bbd3c56c-15e0-4dce-9bae-e5e16a5f7cf2

📥 Commits

Reviewing files that changed from the base of the PR and between d4a3458 and 4180d9a.

📒 Files selected for processing (1)
  • llm/transformer/openai/fake.go

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

The fake OpenAI transformer now uses path.Join to construct fixed and streaming response file paths. The response and error-handling behavior remains unchanged.

Changes

OpenAI fake response paths

Layer / File(s) Summary
Use path.Join for response files
llm/transformer/openai/fake.go
The code replaces the filepath import with path. Do and DoStream now use path.Join for response file paths.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 4180d

This change corrects portable fixture lookup for OpenAI tests without altering response behavior, so it is mergeable.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: using filesystem-compatible paths for embedded OpenAI fixtures.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@looplj
looplj merged commit bd7144e into looplj:unstable Sep 16, 2026
6 checks passed
Edint386 pushed a commit to Edint386/axonhub that referenced this pull request Sep 18, 2026
@remote-controlled-man
remote-controlled-man deleted the fix/windows-embedded-fixtures branch October 13, 2026 01:29
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