Skip to content

Conversation

@subtleGradient
Copy link
Member

…ar files

  • Add createRequestFileKV() for requests that extracts/restores inline base64 data URLs (data:image/png;base64,...) to sidecar files like request.json.assets/0001.png

  • Add createTextFileKV() for response bodies with same binary extraction

  • Update createFilesystemStorage() to use new KV types for requests and response bodies

  • Update test helper findCachedResponse() to handle raw text format

  • Add tests verifying binary extraction/restoration for both requests and responses

…ar files

- Add createRequestFileKV() for requests that extracts/restores inline
  base64 data URLs (data:image/png;base64,...) to sidecar files like
  request.json.assets/0001.png

- Add createTextFileKV() for response bodies with same binary extraction

- Update createFilesystemStorage() to use new KV types for requests and
  response bodies

- Update test helper findCachedResponse() to handle raw text format

- Add tests verifying binary extraction/restoration for both requests
  and responses
Copilot AI review requested due to automatic review settings January 7, 2026 05:05
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: dfd626c3ac

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR implements automatic extraction of inline base64 data URLs (e.g., data:image/png;base64,...) from cached request and response JSON files to separate sidecar binary files, improving cache readability and reducing file bloat.

Key Changes:

  • Introduces createRequestFileKV() and createTextFileKV() functions that automatically extract/restore data URLs to sidecar files (e.g., request.json.assets/0001.png)
  • Updates createFilesystemStorage() to use these new KV implementations for requests and response bodies
  • Updates findCachedResponse() test helper to handle both new (raw text) and old (JSON-wrapped) response formats

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
packages-native/fetch-hooks/src/filesystem-storage.ts Adds createRequestFileKV() and createTextFileKV() functions that integrate binary extraction/restoration using the existing binary-extractor module, and updates createFilesystemStorage() to use these new storage implementations
packages-native/fetch-hooks/test/cache-manager.test.ts Updates findCachedResponse() helper to handle raw text response format and adds comprehensive integration tests verifying binary extraction/restoration for both requests and responses

- Simplify variable handling in createRequestFileKV.set() and createTextFileKV.set()
- Add comprehensive JSDoc documentation for createRequestFileKV and createTextFileKV
- Add test for response body restoration round-trip
- Add changeset for binary sidecar extraction feature
@subtleGradient
Copy link
Member Author

Feedback addressed in 003794f

Copilot suggestions (all addressed):

  1. Redundant variable reassignment - Simplified both createRequestFileKV.set() and createTextFileKV.set() to directly use the extracted content without intermediate variable reassignment.

  2. Missing JSDoc for createRequestFileKV - Added comprehensive JSDoc with @param, @returns, @since, and @example tags.

  3. Missing JSDoc for createTextFileKV - Added comprehensive JSDoc with the same documentation structure.

  4. Missing test for response restoration - Added new test restores data URLs when reading cached response body that verifies round-trip extraction and restoration of data URLs in response bodies.

Codex/chatgpt-codex-connector suggestion (not addressed):

  • Backward compatibility with legacy response.json format - We don't care about backwards compatibility with older cache formats. Users upgrading can clear their cache if needed.

Also added:

  • Changeset file (.changeset/binary-sidecar-extraction.md) documenting this as a minor version bump.

@subtleGradient subtleGradient merged commit e6866e9 into effect-native/main Jan 7, 2026
6 of 9 checks passed
@subtleGradient subtleGradient deleted the fetch-hook-mo-good branch January 7, 2026 05:19
This was referenced Jan 7, 2026
subtleGradient added a commit that referenced this pull request Jan 8, 2026
#207)

* fix(fetch-hooks): extract binary data URLs from request.json to sidecar files

- Add createRequestFileKV() for requests that extracts/restores inline
  base64 data URLs (data:image/png;base64,...) to sidecar files like
  request.json.assets/0001.png

- Add createTextFileKV() for response bodies with same binary extraction

- Update createFilesystemStorage() to use new KV types for requests and
  response bodies

- Update test helper findCachedResponse() to handle raw text format

- Add tests verifying binary extraction/restoration for both requests
  and responses

* refactor(fetch-hooks): address PR feedback - simplify code and add docs

- Simplify variable handling in createRequestFileKV.set() and createTextFileKV.set()
- Add comprehensive JSDoc documentation for createRequestFileKV and createTextFileKV
- Add test for response body restoration round-trip
- Add changeset for binary sidecar extraction feature
subtleGradient added a commit that referenced this pull request Jan 8, 2026
#207)

* fix(fetch-hooks): extract binary data URLs from request.json to sidecar files

- Add createRequestFileKV() for requests that extracts/restores inline
  base64 data URLs (data:image/png;base64,...) to sidecar files like
  request.json.assets/0001.png

- Add createTextFileKV() for response bodies with same binary extraction

- Update createFilesystemStorage() to use new KV types for requests and
  response bodies

- Update test helper findCachedResponse() to handle raw text format

- Add tests verifying binary extraction/restoration for both requests
  and responses

* refactor(fetch-hooks): address PR feedback - simplify code and add docs

- Simplify variable handling in createRequestFileKV.set() and createTextFileKV.set()
- Add comprehensive JSDoc documentation for createRequestFileKV and createTextFileKV
- Add test for response body restoration round-trip
- Add changeset for binary sidecar extraction feature
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: DONE

Development

Successfully merging this pull request may close these issues.

1 participant