-
Notifications
You must be signed in to change notification settings - Fork 348
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Ask Claude to implement encodeResponseBody: "manual"
option.
#3669
base: main
Are you sure you want to change the base?
Conversation
The generated output of |
src/workerd/api/http.c++
Outdated
@@ -1122,7 +1134,7 @@ jsg::Ref<Request> Request::clone(jsg::Lock& js) { | |||
auto bodyClone = Body::clone(js); | |||
|
|||
return jsg::alloc<Request>(method, url, redirect, kj::mv(headersClone), getFetcher(), getSignal(), | |||
kj::mv(cfClone), kj::mv(bodyClone)); | |||
kj::mv(cfClone), kj::mv(bodyClone), cacheMode, responseBodyEncoding); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the absence of cacheMode
here was a bug? The effect would be that .clone()
on a request would not copy over the cacheMode
correctly.
responseBodyEncoding: "manual"
option.encodeResponseBody: "manual"
option.
6c711ec
to
54889c7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm really surprised it did so well.
All of my previous attempts led to absolute gibberish
.
I'm really curious to see the prompt/chat history (didn't find the mentioned comment) I am super curious to see how much prodding and correcting it required to get it right.
Tbf I am extra surprised it was able to write the tests seeing how specific our test syntax is.
@danlapid Sorry I edited the chat transcript link into the PR description but if you only saw the email I guess you missed it. Here it is: https://claude-workerd-transcript.pages.dev/ |
The tests are my favorite part (because I hate writing tests)! The test syntax is unique to this one file, it's using a test harness defined earlier in the file. I guess Claude was able to learn enough from all the other test cases in the file to write its own. |
I wonder if I should also start prepending “You are absolutely right.” every time I respond to someone 😂 |
I've noticed Claude says "You are absolutely right" even when I'm actually wrong, and then it doesn't actually do the wrong thing I said, but makes some other change to distract me. Honestly, brilliant strategy for dealing with humans!
Yes. At the very least, for generating test boilerplate, since I hate that! Not clear yet if it's a win for implementing workerd changes, need to try some other things. There's another project I've been working on (not open source yet) that's a stand-alone TypeScript library written entirely by Claude and its performance there is mind-blowing. But obviously working in workerd is much harder. |
The `Response` encoder has long had a non-standard option `encodeBody: "manual"` which says: "Do not automatically compress the body according to content-encoding; assume the bytes I provide are already compressed." However, this only allowed you to construct a `Response` to return with manual encoding. If you made at outgoing HTTP request, and the response was compressed, there was no way to stop the runtime from automatically decompressing it. This commit adds such a way: setting `encodeResponseBody: "manual"` as an option to `fetch()`. (Of course, `encodeResponseBody` is a non-standard option, but `encodeBody` is as well.) 🚨🚨 THIS PR WAS WRITTEN BY CLAUDE.AI 🚨🚨 (with a lot of prompting by me -- [see trascript](https://claude-workerd-transcript.pages.dev/)) This was an experiment to see how well Claude Code could handle the `workerd` codebase. Final stats: ``` Total cost: $9.77 Total duration (API): 17m 15.5s Total duration (wall): 1h 38m 26.7s ``` These numbers are... quite a bit larger than what I've seen when working with Claude Code on smaller, simpler projects. I am... not really sure I saved much time on the implementation itself, vs. writing it manually. But I am impressed that Claude figured it out! And I especially appreciated it writing the unit test because I hate writing tests.
54889c7
to
fa37d08
Compare
The
Response
encoder has long had a non-standard optionencodeBody: "manual"
which says: "Do not automatically compress the body according to content-encoding; assume the bytes I provide are already compressed."However, this only allowed you to construct a
Response
to return with manual encoding. If you made at outgoing HTTP request, and the response was compressed, there was no way to stop the runtime from automatically decompressing it. This commit adds such a way: settingencodeResponseBody: "manual"
as an option tofetch()
.(Of course,
encodeResponseBody
is a non-standard option, butencodeBody
is as well.)🚨🚨 THIS PR WAS WRITTEN BY CLAUDE.AI 🚨🚨
(with a lot of prompting by me -- see trascript)
This was an experiment to see how well Claude Code could handle the
workerd
codebase. Final stats:These numbers are... quite a bit larger than what I've seen when working with Claude Code on smaller, simpler projects.
I am... not really sure I saved much time on the implementation itself, vs. writing it manually. But I am impressed that Claude figured it out! And I especially appreciated it writing the unit test because I hate writing tests.