Bound buffered relation exports without retaining tracked graphs - #3089
Chris0Jeky wants to merge 6 commits into
Conversation
Reuse the no-tracking relation stream, preserve exported endpoint scope, and reject eligible row 10001 with streaming guidance. Add boundary, cancellation and uncapped-stream controls plus architecture notes for #3069.
|
@codex review Please inspect buffered admission versus streaming, exported same-board endpoint scope, first-overflow refusal and async-enumerator disposal, plus the boundary fixtures. The existing page query is reused, not rewritten. No local .NET run or memory profile is available; exact-head hosted qualification is required. No complete CI or independent review verdict is claimed. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Consolidation checkpoint: a parallel local Taskdeck lane implemented #3069 before this PR appeared. Its alternative is now preserved on branch codex/3069-buffered-relation-limit at3df8038bb662e1cff9ce69a335404f933238589e; the cap implementation itself is commit158aa62bce90828d267ab66d9ad10e624dd02c52. It stays within DataExportService and its existing test class, reusing the same500-row authorized no-tracking projection and10000 admitted-row ceiling. It currently includes the pending #3085 parent and its export compatibility correction. That alternative passed45 focused DataExportService tests plus one actual SQLite/API projection case, and removing the cap made the10001-row regression fail. Independent Terra review is clean. Its full backend/hosted gate has not run. These results apply only to that saved implementation; they do NOT qualify this PR's different collector/service implementation at e5b891c. The owner requested a safe saved checkpoint and session end. No competing PR was opened for the saved alternative. Reconcile the two implementations and their regression coverage before choosing one delivery path; preserve the source and evidence rather than merging both overlapping DataExportService changes. Local receipts and exact restart instructions remain in C:/td0912-evidence/HANDOFF.md and3069-buffered-relations. |
|
Hosted qualification update at The automated review on this head reported no major issues (comment5649305897). The 17 new cases are application/collector controls, not new SQL or memory-profile measurements. Local .NET remains unavailable. Keep draft pending remaining exact-head qualification and integration review; no merge/deployment. |
Chris0Jeky
left a comment
There was a problem hiding this comment.
Fresh-context review: no merge-blocking correctness, authorization, or truncation finding on this head.
The collector composes two independent safety boundaries correctly:
- the existing page reader remains responsible for current account authorization and deterministic page order;
- the buffered collector intersects every edge with the card IDs that were actually admitted to this export, so concurrent access grants/additions cannot introduce foreign or dangling endpoints.
The 10,000-row limit is checked immediately before admission, making row 10,001 a failure rather than a truncated success. Excluded foreign/dangling rows do not consume the budget, the empty-card case avoids enumerating the source, cancellation flows through WithCancellation, and PayloadTooLarge is mapped before the success audit is written. The streaming export path is unchanged and remains the recovery route.
Exact-head Required CI, CI Extended, and Smart CI are completed successfully; a later Extended rerun also succeeded while two redundant siblings were concurrency-cancelled. The documented residuals—offset pagination across multiple reads, no total byte/time cap for this relation slice, and authorization changes between pages—are pre-existing trade-offs and fail toward omission rather than disclosure because of the final exported-card intersection. This PR is ready for maintainer review.
…on-guidance Show relation-specific navigation guidance while proposals are pending
…69/pr-3089-requalify
Outcome
Bound buffered relation exports without retaining tracked graphs.
Buffered account exports now reuse the existing authorized, deterministic, no-tracking relation page reader instead of retaining one tracked graph per board. An internal collector admits at most 10,000 eligible relation DTOs, preserving both endpoints in the already-exported same-board card set. Row 10,001 raises the existing PayloadTooLarge result with streaming guidance; no truncated success or export-success audit is emitted.
The same streaming route stays uncapped by this new budget. Archived authorized endpoints remain portable, owner/BoardAccess checks remain in the existing query, and empty card sets avoid relation reads entirely. No COUNT-then-unlimited-load race, new SQL/repository method, schema, migration or framework. The separate board JSON IncludeCards=false path is untouched; there is no account-export flag invented here.
Architecture and trade-offs
docs/analysis/2026-09-12-buffered-relation-budget.mddocuments the collector boundary, cardinality budget, endpoint scope, failure mapping, deterministic ordering, alternatives and exact validation commands. This bounds retained relation DTOs plus one existing 500-row source page, not total account-export bytes or read duration. Multi-read/offset snapshot limitations and the surrounding cancellation/error mapping remain unchanged.Tests
Thirteen collector cases cover 0/499/500/501/9,999/10,000, overflow at 10,001 and 20,000, foreign/dangling/cross-board endpoints, excluded rows not consuming the eligible budget, empty-card short-circuit, cancellation propagation and disposal on failure. Four service cases run the real export orchestrator with mocked repositories at 9,999/10,000/10,001/20,000; each then streams the same complete dataset. They check no tracked graph reads, no success audit on refusal and no further pages after overflow. The existing archived-card parity test now expects page reads on both routes.
Exact-head verification
Head
e5b891c62edd5e77bfe942c8f5f269a086a8debd:34724525170: success34724525188: success34724525006: success34724670778: success; two redundant siblings were concurrency-cancelled.Isolation
Five files only. Independent of #3084 and #3086. No controller, repository query, proposal/editor, CI-control, canonical STATUS/MASTERPLAN or human acceptance edits. Existing OUTSTANDING_TASKS.md decisions remain unchanged.
Closes #3069