WebUI: table+form rework, system-metadata timestamps, sortable Created/Modified

[leogdion]

Summary

Iterates on top of #329's CloudKit JS mode toggle (PR #335). The single-mode JSON-textarea CRUD grid is replaced by a Notes table beside a Title/Index form; clicking a row loads it for edit, per-row Delete, "New" to clear. Auto-refreshes after every mutation and on mode switch.

• UI — two-column responsive layout (stacks below 820px). Created and Modified columns formatted with locale dateStyle:short/timeStyle:short; ISO in cell tooltip. Created/Modified column headers are clickable: unsorted → ↑ → ↓ → unsorted. Record name is now a row tooltip (no longer a column).
• Note schema — drop the redundant custom createdAt (TIMESTAMP) and modified (INT64) fields; the demo now reads CloudKit system metadata (CKRecord.creationDate / .modificationDate, Web Services created.timestamp / modified.timestamp). Native model, views, integration phases, README, and CLI examples follow.
• Schema additions — explicit ___createTime and ___modTime with QUERYABLE SORTABLE. System fields default to non-sortable; they must be opted in for the new sort feature to work end-to-end.
• Server — new WebJSON.encoder()/.decoder() with .millisecondsSince1970 date strategy so the browser receives epoch millis. WebRequests.Query gains sortBy. WebRequests.Update/.Delete gain optional recordChangeTag. WebRequests.Create/.Update.fields are now [String: FieldValue] decoded through MistKit's existing custom Codable.
• Bug fixes hit while testing the rework — BadRequestException: missing recordChangeTag on delete/update (now threaded through from the loaded row); HTTP 400 on create when sending "index": 5 as a JSON number against the prior [String: String] decoder; Error raised at top level crash on Ctrl+C (WebCommand now catches AsyncTimeoutError.cancelled as a clean shutdown).

Schema deployment

The new ___createTime / ___modTime SORTABLE entries and the dropped createdAt / modified fields need to be deployed to the live CloudKit container manually (cktool or the dashboard) before sort works end-to-end. Default sort is nil, so unsorted listings work against either schema version.

Test plan

• swift test from Examples/MistDemo — 924 tests pass.
• Scripts/lint.sh — exits successfully (swift-format + swiftlint + build + header check all green; trailing periphery warnings are pre-existing on the MistKit core).
• Manual: create a Note with title + numeric index in MistKit mode → row appears, table auto-refreshes.
• Manual: click row, edit title, Save → row updates without recordChangeTag error.
• Manual: per-row Delete → row removed, no BadRequestException.
• Manual: click Modified column → notes resort, ↓ indicator shows; click again → ↑; click again → unsorted. Same flow in CloudKit JS mode after deploying the SORTABLE system fields.
• Manual: Ctrl+C the mistdemo web server → prints Server stopped. and exits 0.

Out of scope

• Public/private DB picker — that's Add public database interface to MistDemoApp and web #275, next on the Interactive MistDemo: web (MistKit vs CloudKit JS) + native app (pure CloudKit framework) #330 roadmap.

🤖 Generated with Claude Code

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2ce08094-c83b-4c4f-bcf1-091671129960

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch webui-improvements

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codecov]

Codecov Report

❌ Patch coverage is 84.96241% with 20 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.30%. Comparing base (1e8b907) to head (4690d03).

Files with missing lines	Patch %	Lines
...Demo/Sources/MistDemoKit/Commands/WebCommand.swift	0.00%	13 Missing ⚠️
...stDemo/Sources/MistDemoKit/Server/WebBackend.swift	0.00%	5 Missing ⚠️
...emoKit/Integration/Phases/ModifyRecordsPhase.swift	0.00%	1 Missing ⚠️
...tDemo/Tests/MistDemoTests/Server/MockBackend.swift	93.75%	1 Missing ⚠️

Additional details and impacted files

@@                     Coverage Diff                      @@
##           330-interactive-mistdemo     #336      +/-   ##
============================================================
+ Coverage                     70.27%   70.30%   +0.03%     
============================================================
  Files                           545      547       +2     
  Lines                         15003    15102      +99     
============================================================
+ Hits                          10543    10618      +75     
- Misses                         4460     4484      +24     

Flag	Coverage Δ
mistdemo-spm-macos	60.08% <84.96%> (+0.28%)	⬆️
mistdemo-swift-6.2-jammy	?
mistdemo-swift-6.2-noble	?
mistdemo-swift-6.3-jammy	?
mistdemo-swift-6.3-noble	60.09% <84.96%> (+0.28%)	⬆️
spm	51.34% <ø> (-0.43%)	⬇️
swift-6.1-jammy	?
swift-6.1-noble	?
swift-6.2-jammy	?
swift-6.2-noble	?
swift-6.3-jammy	?
swift-6.3-noble	51.37% <ø> (+0.05%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[claude]

Code Review — PR #336: WebUI table+form rework, system-metadata timestamps, sortable Created/Modified

Overview

This PR is a well-scoped, purposeful rework of the MistDemo web UI that simultaneously:

• Migrates from custom createdAt/modified schema fields to CloudKit system metadata (___createTime/___modTime)
• Replaces the four-quadrant raw CRUD grid with a table+form pattern
• Fixes three real bugs: missing recordChangeTag on delete/update, HTTP 400 on numeric create fields, and the Ctrl+C crash
• Adds column-level sort cycling (unsorted → ↑ → ↓ → unsorted)

The change set is coherent and the PR description is thorough. Most concerns below are minor.

---

✅ Strengths

Schema cleanup is correct. Dropping createdAt / modified in favour of ___createTime / ___modTime removes redundancy and avoids the class of bugs where client-set and server-set timestamps diverge. The schema additions (QUERYABLE SORTABLE) are required for the sort feature and correctly noted as needing a manual deployment step.

WebJSON.swift is well-designed. Centralising .millisecondsSince1970 in one place ensures the browser always receives epoch-millis that new Date(ms) can consume. The docstring explains the CloudKit JS compatibility reason clearly.

recordChangeTag threading fix is the right approach. The browser holds the tag from the last query; forwarding it directly to update/delete avoids a server-side prefetch round-trip that was never needed.

toDate() in JS is defensive. It handles Date instances, raw numbers, and the { timestamp: … } envelope shape — covering both MistKit-mode and CloudKit JS response shapes cleanly.

XSS hygiene is solid. renderRows() uses textContent and DOM attribute assignment (.title =) throughout — no innerHTML with user content. toISOString() in cell tooltips is also safe.

Test coverage matches the feature scope. New tests for sort forwarding, numeric fields in create, recordChangeTag in update/delete, and the absent-tag nil case are all present and use Swift Testing idioms correctly.

---

Issues & Suggestions

1. deleteNote() routes status to formStatusEl, not tableStatusEl

async function deleteNote(note) {
    clearStatus(formStatusEl);   // ← form panel status
    ...
    setStatus(formStatusEl, `Deleted ${note.recordName}.`, 'success');

Per-row Delete buttons appear in the table, but feedback renders in the form panel on the right. If no note is selected (form is in "New" state), the user may not see the message at all on narrow viewports. Consider routing table-action feedback to tableStatusEl or to the row's own inline cell.

2. No delete confirmation

The per-row Delete button fires immediately. For a demo this is often intentional, but a simple confirm() dialog or at minimum a "Deleted." toast that is visible without the form panel would reduce accidental data loss during demos. At minimum worth a comment that it's intentional.

3. recordChangeTag is optional on the server side for Delete, but CloudKit requires it

// WebRequests.swift
internal struct Delete: Decodable {
    internal let recordType: String
    internal let recordName: String
    internal let recordChangeTag: String?   // optional
}

The PR description says "required by CloudKit Web Services". The server currently passes whatever it receives through — including nil. A client bug (or a future API caller) that omits the tag will get a CloudKit 400 rather than a clear server-side 400. Since this is a demo server, this is low risk, but a guard/validation at the handler level would produce a better error message.

4. MockBackend.flatten() silently drops non-primitive field types

default:
    continue   // asset, date, reference, location, list, etc.

This is fine for current tests, but callers testing those types will get a missing key rather than a test failure with a useful message. Worth a code comment so future contributors know this is intentional.

5. Sort field naming inconsistency between modes (cosmetic)

QuerySortField.field maps to fieldName in the CloudKit JS query:

// MistKit mode
sortBy: [{ field: currentSort.field, ascending: ... }]

// CloudKit JS mode
query.sortBy = [{ fieldName: currentSort.field, ascending: ... }]

No bug here — the property names are correct for each API. But the Swift struct uses field while the CloudKit JS API uses fieldName. A comment on QuerySortField.field noting the CloudKit JS mapping would help future readers.

6. note.recordType || recordTypeInput.value.trim() fallback in deleteNote()

recordType: note.recordType || recordTypeInput.value.trim(),

normalizeRecords() always sets recordType from record.recordType. The fallback is unreachable with the current normalizer. Either document why it's there or remove the dead branch.

---

Test gaps worth considering

• WebJSON.encoder() / WebJSON.decoder() date strategy — not directly tested. A unit test that round-trips a Date through encode→decode and verifies the epoch-millis representation would lock the contract.
• queryForwardsSort test currently asserts the backend receives the sort but not that the response is correctly date-encoded (uses the stub record which has no timestamps). Optional but would catch a regression in WebJSON usage.

---

Minor nits

• WebCommand.swift: the AsyncTimeoutError.cancelled catch is the right fix. Worth also catching a generic CancellationError as a fallback in case the error type changes between Hummingbird versions.
• Examples/MistDemo/schema.ckdb: the trailing blank line between "image" ASSET, and GRANT is new — cosmetically fine but slightly inconsistent with the pre-existing style.

---

Summary

The PR is well-constructed, fixes real bugs, and the new table+form UX is a significant improvement over the raw CRUD grid. The most actionable item is routing per-row Delete feedback to tableStatusEl (issue #1). The rest are informational or low-priority polish. 924 tests passing and lint.sh green are good signals.

[claude]

Code Review — PR #336: WebUI table+form rework, system-metadata timestamps, sortable Created/Modified

Overview

Solid iterative improvement on top of #335. The switch from custom createdAt/modified fields to CloudKit system metadata (___createTime/___modTime) is the right long-term design, the three-bug fix bundle is real and well-targeted, and the test additions are appropriate. A few observations below.

---

Bug Fixes — Well Done

• recordChangeTag threading: The root cause (client had the tag but never sent it; server never forwarded it) is fixed end-to-end across JS → server body → WebBackend protocol → CloudKitService. Both update and delete paths are covered.
• Numeric fields in create: Replacing [String: String] with [String: FieldValue] and removing the stringFields adapter is the clean fix — no intermediate conversion layer to maintain.
• Ctrl+C crash: Catching AsyncTimeoutError.cancelled at the right scope (outside withSignalHandling) and treating it as a clean exit path is correct. The previous code let it propagate to top level.

---

Code Quality

WebJSON.swift — Clean and purposeful. The docstring explains the why (epoch-millis alignment with CloudKit JS wire format) without overstating. One small note: the PR description says "new WebJSON.encoder()/.decoder()" but only the encoder is implemented. If a decoder isn't needed (request bodies don't contain Date fields), the description should be tightened to avoid a future expectation.

WebRequests.QuerySortField — The naming asymmetry between the Swift property (field) and the CloudKit JS property (fieldName) is documented in the struct comment. Acceptable, but worth a constant or a test so a future CloudKit JS field name change is caught in one place. Not a blocker.

normalizeRecords / toDate — The multi-branch timestamp normalization is thorough. CloudKit JS can return a Date object, a number, or { timestamp: Date|number|string }; all three are handled. The null-safety via == null (loose equality covering both null and undefined) at the top of toDate is correct JS idiom.

renderRows — Full tbody.innerHTML = '' re-render on every update is fine for datasets ≤ 200 records, and textContent (not innerHTML) is used for all dynamic data, so no XSS surface is introduced.

deleteNote default parameter — The comment explaining why the default is tableStatusEl is the right call; the two call sites (per-row button vs form Delete) map to different status containers, and the distinction would be invisible without it.

setMode — Good addition of the early return if (mode === currentMode) return; to prevent a spurious re-query when the same mode button is clicked twice.

---

Schema

schema.ckdb correctly opts in ___createTime and ___modTime as QUERYABLE SORTABLE — system fields are not sortable by default, so this is a required change for the sort feature to work end-to-end.

One pre-existing pattern: "image" ASSET, retains a trailing comma before the blank line and GRANT block. This was already the case before this PR. If the CloudKit Schema Language parser were strict about it, deploys would already be failing. Noting it only because the field list changed and it is worth a quick cktool validation when deploying.

---

Tests

Good additions:

• WebJSONTests — pins the epoch-millis contract with a concrete millisecond value.
• WebServerTests+QuerySort — covers both the presence and absence of sortBy.
• Updated CRUD tests — numeric field decoding, recordChangeTag forwarding on update and delete, and the nil-tag case.

Missing coverage worth considering (not blockers for this PR):

• No test verifies that the query response body encodes dates as epoch-millis numbers (i.e., that WebJSON.encoder() is actually wired into the query handler and the browser receives the correct shape). An HTTP-layer test asserting the JSON response contains an integer for a created field would close this loop.
• MockBackend.UpdateCall.fields remains [String: String] (flattened). Tests asserting captured?.fields["index"] == "5" are readable but hide that the protocol received .int64(5). A test that inspects FieldValue directly for the numeric case would give stronger coverage.

---

Minor Points

• Manual test plan items are unchecked — expected at review stage, but worth confirming before merge, especially the per-row Delete and Modified sort flows.
• The refreshFormState() call at script initialization (bottom of <script>) correctly sets the "New note" / disabled Delete state before auth completes. Intentional and correct.
• align-items: end on .table-toolbar aligns the Refresh button to the baseline of the taller label+input combos when the toolbar wraps — intentional flex behavior.

---

Summary

The changes are architecturally sound, the bug fixes are complete, and the new test surface is appropriate. Two items worth addressing before merge:

1. Verify the query response encodes timestamps as epoch-millis end-to-end (test or manual check that new Date(note.created) works for a real record).
2. Tighten the PR description — remove the reference to WebJSON.decoder() since only the encoder is implemented.

Everything else is clean.