alamb commented on issue #23839: URL: https://github.com/apache/datafusion/issues/23839#issuecomment-5156968915
Thanks for filing this @jayzhan211 > It seems large PRs generated by AI is becoming a norm nowadays. SQL query engine is highly structured machine that the output is predictable from the given inputs. Yes, 100% agree. > # What are the GAPs Another gap I think is in how do we find what is most importnt to review (aka use our review bandwidht) - https://github.com/apache/datafusion/issues/21038 > * Correctness coverage > I think we need more test coverage for correctness that gives us high confidence to ship the code if the changes pass the test (e.g., catching silent semantic bugs in complex logic without relying solely on manual test writing). I think the sqllogictesting framework is the best we have in this regard. Running more such tests I think is scalable and adds good end to end coverage. I have found LLms are quite good at creating sqllogictests, so maybe we can spend more time having them expand our existing coverage. For example I have been trying to do this with - https://github.com/apache/datafusion/issues/23307 > * Benchmarking for agents > I'm thinking of more benchmark coverage that is easy to run and reproduce locally (assume most contributors run benchmark in their laptop) and also cost efficient for benchmark bot. This is good -- I tink our bench.sh script is reasonable for this, and @Omega359 's work in this ticket I think will help - https://github.com/apache/datafusion/issues/21706 > * Architecture & System Design > It should be carefully reviewed by reviewers (human). We might need tools to help us easily understand whether changes fit our system design or maybe we need spec in details to tell AI to follow the purpose of each components I think the idea of formalizing / documenting (for humans and agents) what we are looking for in reviews would probably help a lot. We already have https://github.com/apache/datafusion/blob/main/AGENTS.md and https://github.com/apache/datafusion/tree/main/.ai/skills (for ffi that @timsaucer added). Perhaps we can add something similar for performance related or other skills (e.g. that if changing join logic tests should always include nulls, etc). Another idea could be to add a review skill that tries to explain what we are looking for in each PR > > # Objective > This discussion is to gather ideas for how code review could be like in agent era, and what gaps are left for us to achieve the goals. I hope we could ship large PRs without carefully review by human but with high quality code one day. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
