bitflicker64 commented on PR #3424: URL: https://github.com/apache/hugegraph/pull/3424#issuecomment-6507935588
## Multi-reviewer summary (round 1, provisional 2/4) **Verdict:** no confirmed defects introduced by this PR. It looks ready from the reviewed scope. **Reviewed:** FULL review of the whole diff, base `2029e88a` → head `1db1f1bf`, all 4 changed files. Production callers were also checked: `BusinessHandlerImpl`, `QueryUtil`, `MultiListIterator` and the batch loop in `AggregativeQueryObserver.readBatchData`. **Score: 10.0 / 10** (mean of 2 of 4 reviewers; the remaining two are still running) | Reviewer | Raw | Corrections | Adjusted | |---|---|---|---| | Reviewer A (Claude Opus 5.5) | 10 | none | 10 | | Reviewer B (model not reported) | 10 | none | 10 | | Reviewer C | pending | | N/A | | Reviewer D | pending | | N/A | **Unscored notes (non-blocking):** - `seek()` still delegates to the inner iterator while a pending row is buffered. No current query path calls `seek()` after a probe (the only `seek()` in `ScanUtil` runs before the first probe), so this is not a current regression. Consider clearing the pending row if `seek()` ever becomes reachable after a probe. - The `MapLimitIterator` Javadoc for `hasNext`/`next` still describes the old re-check behaviour. **Checks:** `FilterIteratorContractTest` passed 18/18 at head. Run against the base iterators, the new tests fail, so they reproduce the bug. One reviewer did not run Maven locally because its JDK was older than the required one. This complements, and does not repeat, the existing approval. -- 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]
