Repository navigation
Excessive LLM pulls #56
Replies: 9 comments 6 replies
It's not just that. If an author has lots of pull requests all over the place (instead of focusing on some area that they understand in-depth) I will deprioritize reviewing their contributions just based on that, even if they may be impactful - I would likely just be talking to an LLM anyway, which I prefer to do locally. |
|
Thank you, Will, for asking me rather than closing my PRs. I would prefer to keep them open. I know review capacity is the real limit. I co-developed and maintained QuantLib from 2000 to 2015, and today I maintain the 15 btclib-org repositories. To help with the review load, I review and comment on others' PRs: more than 100 across 17 Bitcoin projects, 32 of them on bitcoin/bitcoin and many on the BIPs, libsecp256k1, bitcoinjs-lib and btcd. I would suggest judging each PR by usefulness rather than count. A reviewer who finds one not useful can ignore it, or a maintainer can close it, and I will not mind. One who finds it useful can review it. In three months 12 of my PRs were merged across Core, libsecp256k1, the BIPs, HWI and secp256k1-zkp. I have also sent several private vulnerability reports to Bitcoin projects, with fixes; some are already fixed in releases. btclib-org's libraries implement many BIPs. When their tests reach a case the published vectors miss, I propose it upstream as a new vector. LLMs are among my tools. Using them well takes context, direction, clear rules and many iterations, and that work is mine. I choose every topic and make every decision. Nothing goes out under my name that I have not understood, and I stand behind each PR and comment, so a reviewer talks to me, not to an LLM. We stopped using punch cards, and soon we will stop coding without LLMs. A shorter queue saves reviewers' attention but adds no capacity. LLMs could already carry part of the review load, so that people spend their time where it matters most. I may propose a review bot, like the one I run on btclib-org, to help decide which PRs a human looks at first. |
I have doubts about this. Today you authored more than 15 pull requests. There are a few that fix some small things, but many of them don't make sense: They either concern an interface that is trusted already, document obvious things, ignore a tradeoff that has already been discussed elsewhere in the repo, soften an edge case that doesn't seem worthwhile fixing, or misinterpret existing documentation. As Will said, we treat the pull request list as something that should be fit for human consumption and discovery. We might change that in the future, but that is the current status. So I'd ask you again to prioritise and close the bulk of them.
On review in general in the repo: We have a fairly comprehensive LLM reviewer that is linked in every DrahtBot comment. If you leave review in the future, I would suggest checking it first. If you are interested in tweaking it, see https://github.com/willcl-ark/nix/tree/master/pkgs/ralph . Maybe also target your findings to your own triaging repository first? |
There's too many for me to judge. I want to review your PRs but I need you to help me pick the one that will affect the most users, that's where I'll start. |
|
@fametrano applying a 7 day ban from the repo for now. It's just a rate-limit, a little cool-down, give us humans some time to absorb all your work. You can still leave comments in this meta discussion |
|
I take the concern about the number of my PRs seriously, and I know I was warned in meta#33 on 2 October about bursts of comments. But the ban stops the very work that eases the load: my reviews of other people's open PRs, which should lighten reviewers' work, and acting on the self-review of my open PRs I had been asked to close or re-evaluate. An invitation to steer my productivity towards reviewing open PRs might have worked better than a ban. I opened all these PRs because I judged them all reasonable and worth review. Like anyone, I may be wrong, and checking that is exactly what reviewers and maintainers are for. A PR should be judged on its own merits, not on how prolific its author is. If opening 15 PRs in one day deserves a cool-down ban, would the same 15 PRs, each opened by a different person, have got the same answer? I reviewed all my 35 PRs myself: I keep 30, would close one and leave four undecided. The one to close is #36382, a diagnostic feature rather than a fix. I cannot close it while blocked, so please close it. The four undecided are #36513, where maflcko left the call to hebasto; #36647, an API choice for the kernel maintainers; #36185, test vectors of modest value; and #36639, where ismaelsadeeq asked whether the hardening is worth it. Of the 30, six I first meant to close stay open: #36634, #36652, #36653, #36641, #36640 and #36643. #36634 is a one-line fix whose test fails without it. #36652 fixes a breach of a MUST in RFC 9112 section 6.1: after a request with both Content-Length and Transfer-Encoding, the server must close the connection. I would reduce #36653 and #36641 to what is needed. The other 24 are solid and worth review. @pinheadmz, how many users a change affects is a fair criterion, but it is not the only one and certainly not the main one: a rare correctness bug is still a bug. Review capacity is the real limit, and I think AI tools should help address it today. I offer to help, and I already do so through my reviews of others' open PRs. @pinheadmz, I am human too and I have feelings ;-) Please lift the ban; no hard feelings either way. |
|
@pinheadmz @willcl-ark @sedited, I'd like to ask for the ban to be reconsidered, on the basis of the project's own written rules. Moderation guidelines. The guidelines give moderators a mandate over the content of comments: on-topic, about ideas not people, no generic or personal criticism, no repetition without new information, no meta-discussion in software repos. None of my comments or PRs was challenged on any of these grounds, and to my knowledge none was hidden or deleted. The escalation to timeouts and bans applies "if moderation of individual posts fails"; no individual post of mine was moderated. AI policy. doc/AI_POLICY.md requires that a human knows the language, could have written the code, understands its effects and can explain it in their own words. I do, for every PR, and I have answered every reviewer question myself. The policy sets no limit on the number of PRs or comments, and its remedy for a PR that appears to violate it is closing that PR, not banning its author. No PR of mine was closed on those grounds. Rate limiting. A cap on PRs per day or on open PRs per author appears neither in the guidelines nor in the AI policy. The ban was explicitly described as a rate-limit, i.e. enforcement of a rule that is not written anywhere. If the project wants such a rule, add it to the guidelines and I would follow it. Effect of the ban. It prevents me from doing exactly what I was asked: closing PRs, and answering reviewers on the ones that stay open. I understand review capacity is the real constraint, and the help I can offer is quality review of PRs opened by others. Beyond that, my commitments are the ones in my previous comment: closing #36382, reducing #36653 and #36641 to what is needed, and deciding on the four PRs I left undecided. Thanks for taking the time. |
|
@fametrano If helpful, Bitcoin Core has its own culture on this. An oft-cited rule of thumb has been to review 5 to 15 PRs (or test/handle/help reproduce or solve open issues) for every PR you open. Consider also the time and attention your comments here and your PRs are asking for, relative to other PRs needing attention, perhaps longstanding difficult ones or high priority ones. Please see these articles for more. Best wishes and continue the good work. Cheers. |
|
Thanks, @jonatack, for the rule of thumb and the articles. I accept the rule gladly: from now on I will review 5 to 15 PRs, or help with issues, for each PR I open on bitcoin/bitcoin. So far I am well below it: I have reviewed or commented on 44 PRs by others there, against 42 of my own. That is more attention asked than given, as you point out. My next reviews, of #36360 and #36628, are in preparation; for each I break the code under test and check that the tests catch it. I also owe answers on my open PRs and issues. On #36632 I will drop the first commit, which sedited finds already documented. On #36536 I will add cyb3ralbert's two nits and ask the three reviewers to look again. I will answer Ayoazeez26's note on the maxfeerate failure in #36408. On #36413 I will tell avicdr to go ahead, as I cannot assign the issue. On #36641 I will push the reduction I promised; sedited has closed #36653. @pinheadmz, given this plan, could you lift the ban early? I could then start on these reviews and answers right away. |
that would make it worse, given the speed in which you leave reviews. Semi-automated reviews are even worse than PRs because they are harder to ignore. The point is that for your review to add a benefit you need to get to a point where you have enough understanding that you add extra value over the LLM (otherwise you really wouldn't be needed, the maintainers could just ask the LLM themselves). That means you will typically need to invest hours, sometimes days of manual review for a given non-trivial PR, more if you are new. If you just reproduce LLM output until you think you understand it, you still add nothing of value. So what is needed (in my opinion) is fewer PRs, fewer reviews, and more human thought. |
I agree here. Obviously I love review on my pulls, but if they are basically just an LLM-expanded reword of the existing Ralph LLM review, their value seems close to zero. E.g. bitcoin/bitcoin#36488 (comment) All of this is explained in https://github.com/bitcoin/bitcoin/blob/master/doc/AI_POLICY.md already. An LLM can't help you to understand that, so my recommendation would be to print https://github.com/bitcoin/bitcoin/blob/master/doc/AI_POLICY.md on paper and then read and understand it without access to a computer. |
|
Instead of donating the outputs of large language models, I actually
recommend you donate the credits instead. Then people who are more
experienced than you could trivially use those credits to work on the
project.
|
|
@kanzure, I would rather the work be judged on what it shows than on assumptions about its author. My background is in my first comment. |
|
@mzumsande, I agree that a review is worth posting only if it adds value of its own, and that this takes time. That is the bar for the two reviews I have ready, of #36360 and #36628. On #36360 I ran each mutant in the description through the unit tests on the base, the qa-assets data included. One that the thread says #36130 covers still survives there, although the base includes #36130. The review also adds a test case for a sibling mutant nobody had reported. I will post both when the ban ends and the other I am working at, and they can be judged then. They will be posted all together, hopefully none will complain about the batching :-) |
|
Hi, could you please endeavor to find at least one other Bitcoin developer
that wants to collaborate with you in this manner? You seem to have a
specific workflow in mind, and you're meeting resistance here. It's worth
asking why. If you can find a collaborator, then you can test your workflow
and revise it until they indicate that it's ready for public project
participation. Thanks!
|
Uh oh!
There was an error while loading. Please reload this page.
@fametrano you have opened 13 pull request in the last 12 hours on bitcoin/bitcoin, taking you up to 29 open PRs in total.
With LLMs its possible to find ~ infinite small changes and "bugfixes", many like the pulls you have open, and we simply don't have the human review bandwidth to review them. Some contributors I've talked to have over a thousand fixes from LLM reviews just waiting to be opened.
I am aware that opening infinite LLM bugfixes can work as a process in other projects, often where AI review and/or merging is taking place, or where they have many more reviewers, but our project still has a culture of manual, human review, and flooding the repo with these types of changes doesn't help us in any meaningful way.
What does help us, is having a well-meaning human author in the loop, like yourself, who can keep only the most impactful pulls open against the repo so reviewers can focus their attention on them without having to wade through 5000 open pulls.
I think it would ∴ be best if you would self-close all but say your top 5 or 10 most important pulls, and wait for review on those before opening more. It would be ideal if you could do this using your judgement, as the author of all the changes, vs having a maintainer or moderator just blanket close them.
If they are not seeing review, it's possible other reviewers don't consider them impactful enough to be worth their time. When this happens to me, I'll usually close the PR down after a while and open something else.
With code being essentially free to generate now, there is no limit on the amount that can be produced, but there are still limitations in the review process.
I hope this makes sense to you, let me know if I've not been clear on something.
All reactions