Import deals from file and show Computing… on slow DD solves

[tameware]

Summary

• Add an Import deal… button that loads the first deal from PBN, LIN, DLM, dtest .txt, or sol-style lines (e.g. sol10.txt) into the diagram.
• Show Computing… under the DD matrix after a 300 ms grace period, painted before the blocking WASM ccall so long solves are visible (a timer alone never fires during sync WASM).

Test plan

• bazel test //web:dds_web_js_test //web:dds_web_html_test
• python3 web/serve_web.py → open the page, hard-refresh
• Import hands/example.pbn, hands/list1.txt, a .lin, and hands/sol10.txt; confirm the first deal fills and solves
• Import a multi-deal file; confirm only the first deal is used
• Load a slow deal and confirm Computing… appears under the matrix after ~300 ms, then Solved in N ms.

Made with Cursor

[Copilot]

🟡 Changes recommended

Unresolved critical correctness issues remain in stale solve invalidation and LIN dealer handling.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds deal-file importing and visible progress feedback for slow double-dummy solves.

Changes:

• Supports PBN, LIN, DLM, dtest, and sol-style formats.
• Adds an import button and file input.
• Shows “Computing…” after a 300 ms grace period.
• Adds parser, UI, and timing tests.

File summaries

File	Reviewed changes	Findings
web/tests/test_web_html.py	Import-control markup tests	None noted
web/tests/dds_web_test.mjs	Parser and solve-status tests	None noted
web/dds_web.js	Import parsers, file handling, and solve-status flow	Critical stale-solve invalidation and LIN dealer handling issues; moderate deal validation issue; nit-level error messaging and browser file-selection coverage issues
web/dds_web.html	Import button and file input	None noted

Review details

Suppressed comments (2)

web/dds_web.js:696

• The browser file-selection path is not covered: the tests call importDealFromText directly and only inspect the HTML markup, so regressions in input.files[0], File.text(), or surfacing read/parse errors would pass. Add a JavaScript test for handleDealFileSelected with a fake File covering success and failure.

    const result = document.getElementById("result");
    try {
        const text = await file.text();
        const err = importDealFromText(text);

web/dds_web.js:433

• This shared return path only checks that each direction exists and normalizes strings; it never verifies the four suit fields or uniqueness of the 52 cards. Because normalizeHandHolding filters non-pips first, malformed input with repeated cards or extra/missing suit separators can be reported as a successful import and then sanitized by fillFormWithTestData, leaving altered or incomplete input instead of an import error. Validate the assembled deal before returning it.

        if (!byDirection[direction]) {
            return null;
        }
        deal[direction] = normalizeHandHolding(byDirection[direction]);

• Files reviewed: 4/4 changed files
• Comments generated: 3
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

Unresolved deal validation, stale-status, and asynchronous file-selection issues remain.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (3)

Previously missed (1) — in code that hasn't changed since the last review.

web/dds_web.js:700

• The new browser file-selection path is not covered by the added tests: all import tests call importDealFromText directly, and the HTML test only checks markup. A regression in file.text(), files[0], or the async success/error handler would therefore pass CI while the Import deal button is unusable. Add a unit or browser test that supplies a file through the actual input and verifies both successful import and read/parse errors.

web/dds_web.js:2664

• These guards reject stale WASM work but do not clear stale status. For a complete-deal edit during the wait, updateActionButtons() only arms the 250 ms debounce, so ddTableRequestId can remain unchanged; if the old timer fires, this branch clears the timer and returns while leaving result.innerHTML as Computing… until the debounced refresh starts. Clear the old status when requestId === ddTableRequestId (and apply the same cleanup to the second PBN guard) so the UI does not report computing for the edited deal.

            // An import/edit during the grace wait can change the diagram while
            // this invocation still holds the old PBN; do not solve stale input.
            if (handsToPbn(collectHands()) !== pbn) {
                clearDdTableComputingTimer();
                return;

web/dds_web.js:701

• file.text() is asynchronous, so two quick file selections can complete out of order. This handler imports whichever read resolves last without checking that file is still input.files[0], allowing a slower read of an earlier selection to overwrite the deal chosen afterward. Re-check the current file after the await (or use a selection token) before calling importDealFromText.

        const text = await file.text();
        const err = importDealFromText(text);

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Lite