7 ms·

Taking the question of whether this would be a useful addition to Node.js core or aside, it must be noted that this 19k LoC PR was mostly generated by Claude Co

by indutny 6mo ago

Taking the question of whether this would be a useful addition to Node.js core or aside, it must be noted that this 19k LoC PR was mostly generated by Claude Code and manually reviewed by the submitter which in my opinion is against the spirit of the project and directly violates the terms of Developer's Certificate of Origin set in the project's CONTRIBUTING.md

epolanski 6mo ago
Do as I say, not as I do. On a more serious note, I think that this will be thoroughly reviewed before it gets merged and Node has an entire security team that overviews these.
- indutny 6mo ago
  As someone who was a part of the aforementioned security team I'm not sure I'd be interested in reviewing such volume of machine generated code, expecting trap at every corner. The implicit assumption that I observed at many OSS projects I've been involved with is that first time contributions are rarely accepted if they are too large in volume, and "core contributor" designation exists to signal "I put effort into this code, stand by it, and respect everyone's time in reviewing it". The PR in the post violates this social contract.
  - lemagedurage 6mo ago
    [dead]
  - epolanski 6mo ago
    For free, you can decide to do what you want, if it's your job, it's a bit different and you may have to do so, especially considering Collina, is one of the largest contributors of the project and member of the technical committee.
    - exe34 6mo ago
      > if it's your job, it's a bit different and you may have to do so Oh I'd use an llm to generate large amounts of feedback and request changes!
      - epolanski 6mo ago
        Imagine if every profession reasoned liked that when doing something they don't enjoy.
        kruffalon 6mo ago
        What a wonderful world we would have, or possibly at least better than the current shit show :)
        exe34 6mo ago
        Imagine fighting fire with fire. You don't have to take shit lying down.
        int_19h 6mo ago
        We'd have a lot less enshittification all around, I suspect.
        epolanski 6mo ago
        Sure thing, your nurse ain't gonna clean your mom, in the restaurant the chef ain't gonna prepare a dish he doesn't like, your accountant ain't gonna file your taxes if you've given him data he doesn't like, etc. Your paid to do a job, you're either professional or you aren't.
        lelandfe 6mo ago
        I’d probably leave the hospital if I heard the doctor filed an LLM generated diagnosis for the nurse to validate
        exe34 6mo ago
athorax 6mo ago
How exactly does it violate the Developer's Certificate of Origin clause?
- indutny 6mo ago
  The submitted code must adhere to either of (a), (b), (c), and separately a (d) clause of: https://github.com/nodejs/node/blob/main/CONTRIBUTING.md#developers-certificate-of-origin-11 https://github.com/nodejs/node/blob/main/CONTRIBUTING.md#dev... If submitter picks (a) they assert that they wrote the code themselves and have right to submit it under project's license. If (b) the code was taken from another place with clear license terms compatible with the project's license. If (c) contribution was written by someone else who asserted (a) or (b) and is submitted without changes. Since LLM generated output is based on public code, but lacks attribution and the license of the original it is not possible to pick (b). (a) and (c) cannot be picked based on the submitter disclaimer in the PR body.
  - charcircuit 6mo ago
    It would be considered (a) since the author would own the copyright on the code.
    - crote 6mo ago
      Citation needed. Whether AI output can fall under copyright at all is still up for debate - with some early rulings indicating that the fact that you prompted the AI does not automatically grant you authorship. Even if it does, it hasn't been settled yet what the impact of your AI having been trained on copyrighted material is on its output. You can make a not-completely-unreasonable argument that AI inference output is a derivative work of AI training input. Fact is, the matter isn't settled yet, which means any open-source project should assume the worst possible outcome - which in practice means a massive AI-generated PR like this should be treated like a nuke which could go off at any moment.
      - charcircuit 6mo ago
digikata 6mo ago
Large PRs could follow the practices that the Linux kernel dev lists follow. Sometimes large subsystem changes could be carried separately for a while by the submitter for testing and maintenance before being accepted in theory, reviewed, and if ready, then merged. While the large code changes were maintained, they were often split up into a set of semantically meaningful commits for purposes of review and maintenance. With AI blowing up the line counts on PRs, it's a skill set that more developers need to mature. It's good for their own review to take the mass changes, ask themselves how would they want to systematically review it in parts, then split the PR up into meaningful commits: e.g. interfaces, docs, subsets of changed implementations, etc.
- goalieca 6mo ago
  > With AI blowing up the line counts on PRs, Well, the process you’re describing is mature and intentionally slows things down. The LLM push has almost the opposite philosophy. Everyone talks about going faster and no one believes it is about higher quality.
  - digikata 6mo ago
    Go slow to go fast. Breaking up the PR this way also allows later humans and AI alike to understand the codebase. Slowing down the PR process with standards lets the project move faster overall. If there is some bug that slips by review, having the PR broken down semantically allows quicker analysis and recovery later for one case. Even if you have AI reviewing new Node.js releases for if you want to take in the new version - the commit log will be more analyzable by the AI with semantic commits. Treating the code as throwaway is valid in a few small contexts, but that is not the case for PRs going into maintained projects like Node.js.
  - dotancohen 6mo ago
    > Everyone talks about going faster and no one believes it is about higher quality. Go Fast And Break Things was considered a virtue in the JavaScript community long before LLMs became widely available.
  - tracker1 6mo ago
mixologic 6mo ago
Worth noting that mcollina is a member of the Node.js Technical Steering Committee
- everlier 6mo ago
  We call it a slip slop at work, it's ok to slip some slop if it's "our" slop :-)
  - giancarlostoro 6mo ago
    > I pointed the AI at the tedious parts, the stuff that makes a 14k-line PR possible but no human wants to hand-write: implementing every fs method variant (sync, callback, promises), wiring up test coverage, and generating docs. Is it slop if it is carefully calculated? I tire of hearing people use slop to mean anything AI, even when it is carefully reviewed.
    - grey-area 6mo ago
      Was 14k lines carefully reviewed? Seems unlikely.
      - joshkel 6mo ago
        Considering the many hundreds of technical comments over at the PR (https://github.com/nodejs/node/pull/61478 https://github.com/nodejs/node/pull/61478), the 8 reviewers thanked by name in the article, and the stellar reputations of those involved, seems likely.
        keeganpoppen 6mo ago
        [flagged]
        grey-area 6mo ago
        My mistake 19k lines. At 2 mins per line that’s (19000*2)/60/7=90 7-hour days to review it all, are you sure it was all read? I mean they couldn’t be bothered to write it, so what are the chances they read it all? For someone’s website or one business maybe the risk is worth it, for a widely used software project that many others build on it is horrifying to see that much plausible code generated by an LLM.
syrusakbary 6mo ago
Fully disagree with this take. Not allowing AI assistance on PRs will likely decimate the project in the future, as it will not allow fast iteration speeds compared to other alternatives. Note aside, OpenJS executive director mentioned it's ok to use AI assistance on Node.js contributions: I checked with legal and the foundation is fine with the DCO on AI-assisted contributions. We’ll work on getting this documented. [1]: https://github.com/nodejs/node/pull/61478#issuecomment-4077289061 https://github.com/nodejs/node/pull/61478#issuecomment-40772...
- indutny 6mo ago
  I appreciate hearing your point of view on this. In my opinion the future of Open Source and AI assisted coding is a much bigger issue, and different people have different levels of confidence in both positive and negative outcomes of LLM impact on our industry. It is great to have a legal perspective on compliance of LLM generated code with DCO terms, and I feel safer knowing that at least it doesn't expose Node.js to legal risk. However it doesn't address the well known unresolved ethical concerns over the sourcing of the code produced by LLM tooling.
- szmarczak 6mo ago
  > Not allowing AI assistance on PRs will likely decimate the project in the future, as it will not allow fast iteration speeds compared to other alternatives. It's not an AI issue. Node.js itself is lots of legacy code and many projects depend on that code. When Deno and Bun were in early development, AI wasn't involved. Yes, you can speed up the development a bit but it will never reach the quality of newer runtimes. It's like comparing C to C++. Those languages are from different eras (relatively to each other).
- jaredklewis 6mo ago
  AI coding is great, but iteration speed is absolutely not a desirable trait for a runtime. Stability is everything. Speed code all your SaaS apps, but slow iteration speeds are better for a runtime because once you add something, you can basically never remove it. You can't iterate. You get literally one shot, and if you add a awkward or trappy API, everyone is now stuck with it forever. And what if this "must have" feature turns out to be kind of a dud, because everyone converged on a much more elegant solution a few years later? Congratulations, we now have to maintain this legacy feature forever and everyone has to migrate their codebase to some new solution. Much better to let dependencies and competing platforms like bun or deno do all the innovating. Once everyone has tried and refined all the different ways of solving this particular problem, and all the kinks have been worked out, and all the different ways to structure the API have been tried, you can take just the best of the best ideas and add it into the runtime. It was late, but because of that it will be stable and not a train wreck. But I know what you're thinking. "You can't do that. Just look at what happens to platforms that iterate slowly, like C or C++ or Java. They're toast." Oh wait, never mind, they're among the most popular platforms out there.
conartist6 6mo ago
Pain is a signal. Even if the trick is not minding, it's still inadvisable to burn your hand on an open flame. The pain is there to help you not get hurt. I do not think it is wise to brag that your solution to a problem is extremely painful but that you were impervious to all the pain. Others will still feel it. This code takes bandwidth to host and space on devices and for maintainers it permanently doubles the work associated with evolving the filesystem APIs. If someone else comes along with the same kind of thinking they might just double those doubled costs, and someone else might 8x them, all because nobody could feel the pain they were passing on to others
- nine_k 6mo ago
  I don't see it to be such a pain. > Bundle a full application into a Single Executable. Embed a zip file into the executable, or something. Node sort of supports this since v25, see --build-sea. Bun and Deno support this for a longer time. > Run tests without touching the disk. This must be left to the host system to decide. Maybe I want them to touch the disk and leave traces useful for debugging. I'd go with tmpfile / tmpdir; whoever cares, knows to mount them as tmpfs, which sits in RAM. (Or a ramdisk under Windows.) > Sandbox a tenant’s file access. In a multi-tenant platform, you need to confine each tenant to a directory without them escaping This looks like a wrong tool, again. Run your Node app in a container (like you are already doing), mount every tenant's directory as a separate mount point into your container. (Similar with BSD jails.) This seems like the only problem that is not trivial to solve without a "VFS", but I'm not very certain that such a VFS would be as well-audited as Docker, or nsenter and unshare. The amount of work necessary for implementing that is too much for the niche benefit it would provide. > Load code generated at runtime. See tmpfs for a trivial answer. For a less trivial answer, I don't see how Node's code loader is bound to a filesystem. If it can import via https, Just use ESM loader hooks and register() your loader, assuming you're running Node ≥ 20.6.
madeofpalk 6mo ago
> it must be noted that this 19k LoC PR was mostly generated by Claude Code and manually reviewed by the submitter Who reviewed and approved the PR?
petetnt 6mo ago
Personally I’d like to thank you for raising the point, it seems that tsc members are willing to ram the PR through regardless as per jasnell’s LLM analysis that honestly seems like a hostile gish galloping attempt than an actual honest analysis.
deleted 6mo ago
[deleted]
AgentNode 6mo ago
[flagged]
ovflowd 6mo ago
Matteo wrote a pretty neat article that effective counters your claim https://adventures.nodeland.dev/archive/who-is-responsible-for-ai-generated-code/ https://adventures.nodeland.dev/archive/who-is-responsible-f...
- dannyfritz07 6mo ago
  Am I reading this right that Matteo is saying providence is not important because there are lots of historical cases of not having providence of code? > Many contributions contain routine, non-copyrightable material, and developers still sign off on them. > Compilers change code in ways developers do not always track. Template generators create output from their own logic. Stack Overflow answers are often copied into codebases without much thought about licensing.

by indutny 6mo ago

epolanski 6mo ago

Do as I say, not as I do. On a more serious note, I think that this will be thoroughly reviewed before it gets merged and Node has an entire security team that overviews these.

indutny 6mo ago
As someone who was a part of the aforementioned security team I'm not sure I'd be interested in reviewing such volume of machine generated code, expecting trap at every corner. The implicit assumption that I observed at many OSS projects I've been involved with is that first time contributions are rarely accepted if they are too large in volume, and "core contributor" designation exists to signal "I put effort into this code, stand by it, and respect everyone's time in reviewing it". The PR in the post violates this social contract.
- lemagedurage 6mo ago
  [dead]
- epolanski 6mo ago
  For free, you can decide to do what you want, if it's your job, it's a bit different and you may have to do so, especially considering Collina, is one of the largest contributors of the project and member of the technical committee.
  - exe34 6mo ago
    > if it's your job, it's a bit different and you may have to do so Oh I'd use an llm to generate large amounts of feedback and request changes!
    - epolanski 6mo ago
      Imagine if every profession reasoned liked that when doing something they don't enjoy.
      - kruffalon 6mo ago
        What a wonderful world we would have, or possibly at least better than the current shit show :)
      - exe34 6mo ago
        Imagine fighting fire with fire. You don't have to take shit lying down.
      - int_19h 6mo ago
        We'd have a lot less enshittification all around, I suspect.
        epolanski 6mo ago
        Sure thing, your nurse ain't gonna clean your mom, in the restaurant the chef ain't gonna prepare a dish he doesn't like, your accountant ain't gonna file your taxes if you've given him data he doesn't like, etc. Your paid to do a job, you're either professional or you aren't.
        lelandfe 6mo ago
        I’d probably leave the hospital if I heard the doctor filed an LLM generated diagnosis for the nurse to validate
        exe34 6mo ago

athorax 6mo ago

How exactly does it violate the Developer's Certificate of Origin clause?

indutny 6mo ago
The submitted code must adhere to either of (a), (b), (c), and separately a (d) clause of: https://github.com/nodejs/node/blob/main/CONTRIBUTING.md#developers-certificate-of-origin-11 https://github.com/nodejs/node/blob/main/CONTRIBUTING.md#dev... If submitter picks (a) they assert that they wrote the code themselves and have right to submit it under project's license. If (b) the code was taken from another place with clear license terms compatible with the project's license. If (c) contribution was written by someone else who asserted (a) or (b) and is submitted without changes. Since LLM generated output is based on public code, but lacks attribution and the license of the original it is not possible to pick (b). (a) and (c) cannot be picked based on the submitter disclaimer in the PR body.
- charcircuit 6mo ago
  It would be considered (a) since the author would own the copyright on the code.
  - crote 6mo ago
    Citation needed. Whether AI output can fall under copyright at all is still up for debate - with some early rulings indicating that the fact that you prompted the AI does not automatically grant you authorship. Even if it does, it hasn't been settled yet what the impact of your AI having been trained on copyrighted material is on its output. You can make a not-completely-unreasonable argument that AI inference output is a derivative work of AI training input. Fact is, the matter isn't settled yet, which means any open-source project should assume the worst possible outcome - which in practice means a massive AI-generated PR like this should be treated like a nuke which could go off at any moment.
    - charcircuit 6mo ago

digikata 6mo ago

Large PRs could follow the practices that the Linux kernel dev lists follow. Sometimes large subsystem changes could be carried separately for a while by the submitter for testing and maintenance before being accepted in theory, reviewed, and if ready, then merged. While the large code changes were maintained, they were often split up into a set of semantically meaningful commits for purposes of review and maintenance. With AI blowing up the line counts on PRs, it's a skill set that more developers need to mature. It's good for their own review to take the mass changes, ask themselves how would they want to systematically review it in parts, then split the PR up into meaningful commits: e.g. interfaces, docs, subsets of changed implementations, etc.

goalieca 6mo ago
> With AI blowing up the line counts on PRs, Well, the process you’re describing is mature and intentionally slows things down. The LLM push has almost the opposite philosophy. Everyone talks about going faster and no one believes it is about higher quality.
- digikata 6mo ago
  Go slow to go fast. Breaking up the PR this way also allows later humans and AI alike to understand the codebase. Slowing down the PR process with standards lets the project move faster overall. If there is some bug that slips by review, having the PR broken down semantically allows quicker analysis and recovery later for one case. Even if you have AI reviewing new Node.js releases for if you want to take in the new version - the commit log will be more analyzable by the AI with semantic commits. Treating the code as throwaway is valid in a few small contexts, but that is not the case for PRs going into maintained projects like Node.js.
- dotancohen 6mo ago
  > Everyone talks about going faster and no one believes it is about higher quality. Go Fast And Break Things was considered a virtue in the JavaScript community long before LLMs became widely available.
- tracker1 6mo ago

mixologic 6mo ago

Worth noting that mcollina is a member of the Node.js Technical Steering Committee

everlier 6mo ago
We call it a slip slop at work, it's ok to slip some slop if it's "our" slop :-)
- giancarlostoro 6mo ago
  > I pointed the AI at the tedious parts, the stuff that makes a 14k-line PR possible but no human wants to hand-write: implementing every fs method variant (sync, callback, promises), wiring up test coverage, and generating docs. Is it slop if it is carefully calculated? I tire of hearing people use slop to mean anything AI, even when it is carefully reviewed.
  - grey-area 6mo ago
    Was 14k lines carefully reviewed? Seems unlikely.
    - joshkel 6mo ago
      Considering the many hundreds of technical comments over at the PR (https://github.com/nodejs/node/pull/61478 https://github.com/nodejs/node/pull/61478), the 8 reviewers thanked by name in the article, and the stellar reputations of those involved, seems likely.
      - keeganpoppen 6mo ago
        [flagged]
      - grey-area 6mo ago
        My mistake 19k lines. At 2 mins per line that’s (19000*2)/60/7=90 7-hour days to review it all, are you sure it was all read? I mean they couldn’t be bothered to write it, so what are the chances they read it all? For someone’s website or one business maybe the risk is worth it, for a widely used software project that many others build on it is horrifying to see that much plausible code generated by an LLM.

syrusakbary 6mo ago

Fully disagree with this take. Not allowing AI assistance on PRs will likely decimate the project in the future, as it will not allow fast iteration speeds compared to other alternatives. Note aside, OpenJS executive director mentioned it's ok to use AI assistance on Node.js contributions: I checked with legal and the foundation is fine with the DCO on AI-assisted contributions. We’ll work on getting this documented. [1]: https://github.com/nodejs/node/pull/61478#issuecomment-4077289061 https://github.com/nodejs/node/pull/61478#issuecomment-40772...

indutny 6mo ago
I appreciate hearing your point of view on this. In my opinion the future of Open Source and AI assisted coding is a much bigger issue, and different people have different levels of confidence in both positive and negative outcomes of LLM impact on our industry. It is great to have a legal perspective on compliance of LLM generated code with DCO terms, and I feel safer knowing that at least it doesn't expose Node.js to legal risk. However it doesn't address the well known unresolved ethical concerns over the sourcing of the code produced by LLM tooling.
szmarczak 6mo ago
> Not allowing AI assistance on PRs will likely decimate the project in the future, as it will not allow fast iteration speeds compared to other alternatives. It's not an AI issue. Node.js itself is lots of legacy code and many projects depend on that code. When Deno and Bun were in early development, AI wasn't involved. Yes, you can speed up the development a bit but it will never reach the quality of newer runtimes. It's like comparing C to C++. Those languages are from different eras (relatively to each other).
jaredklewis 6mo ago
AI coding is great, but iteration speed is absolutely not a desirable trait for a runtime. Stability is everything. Speed code all your SaaS apps, but slow iteration speeds are better for a runtime because once you add something, you can basically never remove it. You can't iterate. You get literally one shot, and if you add a awkward or trappy API, everyone is now stuck with it forever. And what if this "must have" feature turns out to be kind of a dud, because everyone converged on a much more elegant solution a few years later? Congratulations, we now have to maintain this legacy feature forever and everyone has to migrate their codebase to some new solution. Much better to let dependencies and competing platforms like bun or deno do all the innovating. Once everyone has tried and refined all the different ways of solving this particular problem, and all the kinks have been worked out, and all the different ways to structure the API have been tried, you can take just the best of the best ideas and add it into the runtime. It was late, but because of that it will be stable and not a train wreck. But I know what you're thinking. "You can't do that. Just look at what happens to platforms that iterate slowly, like C or C++ or Java. They're toast." Oh wait, never mind, they're among the most popular platforms out there.

conartist6 6mo ago

Pain is a signal. Even if the trick is not minding, it's still inadvisable to burn your hand on an open flame. The pain is there to help you not get hurt. I do not think it is wise to brag that your solution to a problem is extremely painful but that you were impervious to all the pain. Others will still feel it. This code takes bandwidth to host and space on devices and for maintainers it permanently doubles the work associated with evolving the filesystem APIs. If someone else comes along with the same kind of thinking they might just double those doubled costs, and someone else might 8x them, all because nobody could feel the pain they were passing on to others

nine_k 6mo ago
I don't see it to be such a pain. > Bundle a full application into a Single Executable. Embed a zip file into the executable, or something. Node sort of supports this since v25, see --build-sea. Bun and Deno support this for a longer time. > Run tests without touching the disk. This must be left to the host system to decide. Maybe I want them to touch the disk and leave traces useful for debugging. I'd go with tmpfile / tmpdir; whoever cares, knows to mount them as tmpfs, which sits in RAM. (Or a ramdisk under Windows.) > Sandbox a tenant’s file access. In a multi-tenant platform, you need to confine each tenant to a directory without them escaping This looks like a wrong tool, again. Run your Node app in a container (like you are already doing), mount every tenant's directory as a separate mount point into your container. (Similar with BSD jails.) This seems like the only problem that is not trivial to solve without a "VFS", but I'm not very certain that such a VFS would be as well-audited as Docker, or nsenter and unshare. The amount of work necessary for implementing that is too much for the niche benefit it would provide. > Load code generated at runtime. See tmpfs for a trivial answer. For a less trivial answer, I don't see how Node's code loader is bound to a filesystem. If it can import via https, Just use ESM loader hooks and register() your loader, assuming you're running Node ≥ 20.6.

madeofpalk 6mo ago

> it must be noted that this 19k LoC PR was mostly generated by Claude Code and manually reviewed by the submitter Who reviewed and approved the PR?

petetnt 6mo ago

Personally I’d like to thank you for raising the point, it seems that tsc members are willing to ram the PR through regardless as per jasnell’s LLM analysis that honestly seems like a hostile gish galloping attempt than an actual honest analysis.

deleted 6mo ago

[deleted]

AgentNode 6mo ago

[flagged]

ovflowd 6mo ago

Matteo wrote a pretty neat article that effective counters your claim https://adventures.nodeland.dev/archive/who-is-responsible-for-ai-generated-code/ https://adventures.nodeland.dev/archive/who-is-responsible-f...

dannyfritz07 6mo ago
Am I reading this right that Matteo is saying providence is not important because there are lots of historical cases of not having providence of code? > Many contributions contain routine, non-copyrightable material, and developers still sign off on them. > Compilers change code in ways developers do not always track. Template generators create output from their own logic. Stack Overflow answers are often copied into codebases without much thought about licensing.