Repository navigation
Conversation
willem-bd
left a comment
There was a problem hiding this comment.
Read-only static review of pinned current revision; verified introduced findings below. No PR code or tests executed.
|
@ggnnggez @Totoro-qaq @AnnaSuSu @willem-bd Follow-up validation is complete for the published head The current diff completes the native batch panel, with the existing owner-scoped router and report/link/image/source renderers. The separate plugin/public contract/parser and unrelated ChatBox/sidecar/mobile changes are withdrawn. All nine inline comments have concrete fixes and replies; I rechecked the current implementation with a separate AI reviewer and have marked those addressed threads resolved. This does not imply an approving review.
The E2E SHA differs only in The description is updated with these completed results. Official PR CI is still |
AnnaSuSu
left a comment
There was a problem hiding this comment.
Reviewed head 0b0e4dc0bfd080ae62b7e2adf572f15114f309b5 against base 5ecc1c2ac2de624af354bc515d5cb04f80df226c.
The revised implementation fits the native batch workflow: selected-result inspection reuses the existing owner-scoped router and report, link, image and citation components. Saved evidence remains isolated from surrounding conversation sources, and report/source dialogs are removed on scope changes. I found no blocking functional regressions in the reviewed scope.
Independent local validation:
- Backend projection, real SQLite HTTP admission, persisted evidence, model-facing reader and batch service/router regressions: 137 passed, 18 skipped.
- Frontend report/citation tests and shared renderer compatibility checks: 94 passed.
- Native browser coverage for default/Custom Agent chats, desktop/mobile, and denied HTTP reads: 5 passed.
pnpm check, production build, changed-file Ruff lint/format and diff checks passed.
The PostgreSQL-gated cases were not exercised locally, and I did not rerun the complete backend suite. Browser coverage uses synthetic authentication and mocked artifact-image serving; output-file checks verify the target URL rather than a real artifact download.
The official PR workflows still await maintainer approval to run. Please complete those checks before merging.
|
@ggnnggez The browser corrections are now submitted separately in #6554, as requested. #6509's published head remains The standalone frontend fix passes the unchanged hosted default/auth E2E suites (394 / 5, zero failures or flakes), frontend unit suite (2,772), lint/types/production build and real-backend replay. node2 additionally passes all 15 focused browser cases with retries disabled, including the original exact scroll assertions and rapid reopen. Its full combined browser run is in progress after freeing completed build caches. For integration verification, #6554's main-based backend shard 4 still hits the cancellation-test hang tracked by #6552; it is disclosed separately. Production shutdown logic is unchanged here. I will synchronize #6509 after the independent upstream fixes are accepted and rerun the affected checks against the resulting published head. |
|
@AnnaSuSu Thanks for the independent review and approval. Your validation boundaries are retained in the description; I have not treated mocked browser artifact serving as a real report-file download. The upstream #6552 fix is now included. GitHub's automatic branch update misplaced the old test-only synchronization in a new lifecycle test; At the exact published head Complete hosted backend/PostgreSQL validation is running again for the updated revision. The independent browser corrections remain in #6554, and the complete node2 browser/auth/replay validation of the separately identified integration head is in progress. Official PR workflows still require maintainer approval; I will report their actual status separately and keep the latest full-suite results bound to their tested SHAs. |
|
@AnnaSuSu @ggnnggez Final update after merging official main The complete current-head backend/PostgreSQL 17/Redis 7 workflow passes 27,374 tests / 105 conditional skips. Current frontend unit tests (2,808), strict blocking-I/O (235), lint/guidance/types/format/production build and replay/golden all pass. #6509's newly executed full default/auth browser workflow passes 398 / 5 with zero failures/flakes. node2's fresh native/affected-upstream matrix at The complete node2 default/auth/replay browser 399 / 5 / 4, all 2,810 frontend tests and check/format passed at The original scroll/transform/order assertions are retained; the missing-row synchronization still rejects the first complete reversed ordering. Synthetic report auth/image/href coverage remains disclosed separately from real artifact HTTP response-byte testing. All nine #6509 inline threads remain resolved; AnnaSuSu's approval applies to the earlier reviewed native implementation. Official upstream workflows still need maintainer approval. The independent #6554 remains separate; #6509 will receive it after upstream acceptance. Integration note: #6554 can land independently of #6509. My proposed integration sequence is to let #6554 land when ready, then I will update #6509 from main and rerun its complete validation before the batch feature is considered for acceptance. I will handle those follow-up steps; landing order and timing remain at maintainer discretion. This relationship is now stated at the top of both PR descriptions. |
|
Resolved the router-guide merge conflict at Independent review confirms the contribution's code patches are unchanged against the new base and upstream goal/focus guards are preserved. Focused validation: backend 97 passed / 2 PostgreSQL-conditional skips, frontend 80 passed, The six official workflows for the new head are still awaiting maintainer approval. #6554 remains a separate main-based browser fix; after it lands I will update this branch, run the affected local checks and use the official full CI for merge validation. |
|
@willem-bd @ggnnggez A quick follow-up when you have time: all nine inline review threads have been addressed and resolved, and GitHub currently reports #6509 as conflict-free. The unrelated browser/layout fixes were split into #6554 as requested. Its official backend/frontend unit tests, E2E, Replay and lint checks have now passed. The six official validation workflows for #6509's current head ( Could you take another look and, if appropriate, approve those workflows so the current head can receive upstream validation? I will handle updating #6509 from main and the affected checks after the separate fix lands. Thanks for your time and review. |
|
@WillemJiang Could you help with maintainer review and CI approval when you have time? #6509 now completes report/evidence inspection in the native batch panel. All nine inline review threads are resolved, the branch is conflict-free, and Totoro-qaq has approved the current head ( As requested during review, the unrelated browser/layout fixes are separate in #6554. Its official unit, E2E, Replay and lint workflows have passed, and it is awaiting review. I will handle synchronizing #6509 and the affected checks after that fix lands. Prior validation is linked by its tested SHA in the descriptions. Thanks for your time. |
|
Closing this PR as requested so the earlier batch-review/browser work is no longer left open. Its validation and review history remain available in the PR; no merge is requested. |
|
Closing this PR as requested. |
Closes #6440
Relationship to #6554
These are separate, main-based PRs, split as requested in review. #6554 fixes existing browser/layout regressions and can land independently of #6509; it does not contain the batch-report feature.
For the final combined validation, my proposed sequence is:
main.maininto feat(batch): inspect saved reports and captured evidence #6509, rerun affected local checks and use the official full CI for merge validation.main.I will handle the branch update and validation. This is a proposed integration sequence, not a request to bypass review or official CI; maintainer preference determines landing order and timing.
Why
Durable batches retain reports, acceptance verdicts and captured RAGFlow evidence, but the native batch panel only exposes previews and bulk export. Returning to a research conversation should let a user inspect one saved report and its original evidence without rerunning the agent.
This delivers the browser consumer proposed in #6440, following #6413 and #6416. The RFC establishes scope; it does not imply maintainer approval.
What changed
Batches → View items → View report opens the selected saved result in the existing default or Custom Agent conversation, on desktop and mobile. The action appears only for terminal items or items with a stored preview; pagination and retry stay in the native batch panel.
Reports reuse
SafeStreamdown, the native Markdown link renderer, shared chat artifact images and knowledge-source dialogs./mnt/output links/images resolve through the current thread's artifact route. Relative image paths do not borrow the parent conversation's artifact inventory. Report HTML stays inert.Execution and acceptance are separate. Missing/empty criteria show No criteria; Unverified means existing criteria lack conclusive verification, including checker errors. Checked/holds/detail outcomes are shown per criterion. Missing reports, read failures and truncation have explicit states.
The selected-result GET uses the existing owner-scoped batch router and
threads:readadmission, including with the worker stopped. It reads one immutable position and projects a bounded response off the request event loop. The HTTP/model reader share their internal item-field allowlist; the model-facing output is unchanged. Prompts, execution specifications, leases and arbitrary artifacts are excluded.Only the selected result's captured evidence can satisfy citations. Saved evidence with blank names/excerpts is rejected, while live-chat citation collection retains its existing empty-field behavior. Neither providers nor the agent are rerun, and parent sources cannot fill missing saved evidence.
Open nonterminal results refresh until terminal. Closing or changing user/thread/item cancels obsolete reads and removes the report/source dialogs; retries invalidate cached results, and new revisions close obsolete evidence. The one-million-character report ceiling and whole-excerpt limits remain enforced.
The separate extension, handwritten parser, duplicate authorization path, public extension contract and version bump are withdrawn. The unrelated ChatBox panel-width fix and sidecar/mobile E2E edits are also withdrawn for separate follow-up. No contribution-specific dependency or schema changes are included; upstream dependency upgrades are retained in the merge.
Surface area
Frontend UI — native batch report dialog and reused report/link/image/source components
Backend API — selected-result read in the existing batch router
Agents / LangGraph — no graph, prompt or model-facing channel changes
Sandbox
Skills
Dependencies
Default behavior change — native batch report inspection
Docs / tests / CI only — runtime changes are included
Screenshots / Recording
Native browser captures show the conversation entry, saved report and captured source. The batch HTTP fixture uses the real SQLite repository/native router with synthetic test authentication and a stopped worker; its example chart is a synthetic browser-route fixture. File-link checks verify the target href; the synthetic chart actually loads at 80px. These captures do not verify downloading a real report.md or the production artifact-serving chain.
Bug fix verification
The deleted plugin's list-contained tilde fence activated a citation inside code and suppressed the following citation. Native DOM/browser regressions cover that structure and an unclosed quoted fence: code stays literal and genuine citations remain actionable on default/Custom Agent routes and desktop/mobile.
The source validation, route authorization, retry/revision cancellation and million-character plain-text regressions are included in the complete suites below. No test assertion or skip was relaxed. These deterministic checks establish application behavior; they do not claim improved model accuracy or complex-Markdown performance.
Validation
Latest conflict resolution — 2026-10-10
Published head:
c6462e7ef04993153872e91730a844c65b8c60b5, incorporating official maindb9b0b4b. Remote contributor updates throughd9157f18were preserved before merging upstream. The only manual conflict resolution isbackend/app/gateway/routers/AGENTS.md: retain upstream MCP committed-reconciliation/drained-cleanup and custom-skill locking guidance, then retain the saved native batch report contract.The upstream sync also carries runtime/UI changes, including goal edit guards. Independent review verified that the contribution's 30 text-file patches against the new base are unchanged, the shared image extraction retains upstream behavior, and the goal/focus guards survive the automatic merge. No browser-fix changes from #6554 were folded into this PR.
pnpm checkThis update used affected-path checks rather than repeating the full local suites. Earlier full-suite evidence below retains its actual SHA and is not relabeled as testing this new head. All six official workflows at this head remain
action_required, awaiting maintainer approval.Previous completed full-suite evidence
The final upstream merge includes MCP cache reconciliation, E2B filename quoting and Windows file-URI test corrections. Current-head full hosted backend validation and replay are complete and green. node2 also passes a fresh 4-case real-backend replay at integration
3b606f19, with retries disabled. The completed browser results below retain their actual tested SHAs; frontend and native batch fixture/router/projection source are unchanged by this merge. The updated integration head is3b606f19. Its complete hosted backend, frontend, blocking-I/O, lint/types/build and replay checks also pass. The complete node2 399/5/4 browser and 2,810-unit result remains at its actual SHAc1a75104, with byte-identical frontend and native batch fixture/router/projection/service; it is not relabeled as a newly executed full default/auth browser run.Previously validated native head:
fc150be0fa1b3f64ea201643c38c545b44a8f2eb, incorporating official main19befe76a6a7636a47c18e34be65ca01c572793dwith ordinary merges. The native report implementation is unchanged by the final upstream merges. Upstream #6552 now supplies cancellation-safe cleanup; the superseded test synchronization has been removed by restoring the lifecycle test byte-for-byte to upstream. All nine inline review threads have implementation/reply evidence and are resolved. AnnaSuSu approved the earlier native implementation at0b0e4dc0; that approval is not presented as a review of subsequent upstream merges.The independent browser reliability changes are submitted separately in #6554, per review. The integration branch combines the two only for validation. #6509 will receive those fixes after upstream accepts them.
The personal-fork PR uses the unmodified existing workflows and the trusted official base/head range. It is CI verification rather than official upstream approval. The initial branch-creation lint failure came from checking every inherited guidance chain; the actual PR-range check passes without changing a limit, workflow, assertion or skip.
fc150be0fc150be0, PostgreSQL-gated SQL/HTTP/evidence/artifact plus affected upstream pathsc1a75104, including #6554node2 uses an isolated Ubuntu 24.04 container with Actions Python 3.12.15, locked uv dependencies including PostgreSQL extras, Node 24.21.0, pnpm 10.26.2 and pinned Playwright Chromium 1217, PostgreSQL 17.11 and Redis 7.4.11. Its latest full browser validation disables retries; only the production web-server startup allowance is extended to 900 seconds for VM resources. Browser authentication and report-image serving are synthetic as described above. Real artifact-router response-byte tests are additional HTTP coverage and do not turn the browser href/image checks into a real end-to-end file download.
Earlier runs and diagnostic failures are retained in the local evidence archive; the current result table supersedes the older validation matrix. Conditional skips are reported explicitly and are not counted as passes. Official upstream Unit Tests, Frontend Unit Tests, Lint Check, Backend Blocking IO, E2E and Replay E2E still need maintainer approval to run. Labels and CLA are separate checks. Official suite success is not claimed, and neither merge nor auto-merge is enabled.
Three PNG screenshots are additional binary files; upstream merge changes are excluded.
AI assistance
Tool(s) used: OpenAI Codex.
How you used it: Codex reproduced the issues, implemented the fixes and regression tests, ran validation, and prepared this description. A separate AI reviewer examined the changes. Human review responsibility remains with the contributor.