| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
|
Hi, this is obviously vibe-coded, so what's the motivation behind this? Have you considered what impact this would have on existing SDK users? Additionally - what value do you see in this unit test? |
Sorry, something went wrong.
|
Fair feedback. After looking at this again, I should clarify the motivation better. The original trigger was noticing a performance difference between StreamReader and ResultsReader, so I initially framed this as a small performance-oriented cleanup. I looked for a larger issue and did not find one, but this read path seemed like a reasonable narrow candidate. That said, the motivation is not just raw performance. The current readlines() call can materialize the whole JSON result stream before yielding the first item, while JSONResultsReader is intended to behave as a streaming iterator for potentially large result sets. The intended behavior change is to preserve the same parsed output while consuming the buffered stream line by line. For SDK users, the intent is to keep result ordering, messages, preview state, and parsing behavior the same, while avoiding a full readlines() pass. I agree that the current unit test is not strong enough. It verifies the basic parsed output, but it mostly proves that the implementation avoids readlines(), rather than proving stronger streaming behavior from a real buffered stream. I can revise the PR description and test around that. If you feel this change is not worth taking, I’m also fine with closing it. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Summary
Testing