fix: address coordinate review findings

Issue: #224
User-Visible: no
This commit is contained in:
Sergey Matyunin
2026-08-22 15:32:55 +03:00
parent 51fde854e3
commit 4dbdb446f8
10 changed files with 107 additions and 32 deletions
File diff suppressed because one or more lines are too long
File diff suppressed because one or more lines are too long
+2 -2
View File
File diff suppressed because one or more lines are too long
+11 -11
View File
@@ -1,7 +1,7 @@
{
"version": 1,
"fixture": "synthetic-only",
"sourceFingerprint": "ed33e779c76b7373c152ecf13e82064a09fe62608af1c75cea96c48369cfec11",
"sourceFingerprint": "ef5431f42e2c08425388d4d14df10de29040ea755ac64f81b734430729faae4f",
"captureScriptSha256": "34f2219790d46efd8250e7a1bd829cb8fc0b0547e1260635fefa52407551b41b",
"command": "npm run build && node demo/docs/capture.mjs",
"scenarios": {
@@ -13,7 +13,7 @@
},
"theme": "dark",
"language": "en",
"sourceSha256": "ed33e779c76b7373c152ecf13e82064a09fe62608af1c75cea96c48369cfec11",
"sourceSha256": "ef5431f42e2c08425388d4d14df10de29040ea755ac64f81b734430729faae4f",
"imageSha256": "2885f96e348b15ab7c696e56e99bddcd9bb2ee94218a883e5a2155f45f5042aa"
},
"view-touch": {
@@ -24,7 +24,7 @@
},
"theme": "dark",
"language": "en",
"sourceSha256": "ed33e779c76b7373c152ecf13e82064a09fe62608af1c75cea96c48369cfec11",
"sourceSha256": "ef5431f42e2c08425388d4d14df10de29040ea755ac64f81b734430729faae4f",
"imageSha256": "f62d8af3617c00a5e99511bd765980d2e27badf25047a1be34046b64195abe6c"
},
"space-create": {
@@ -35,7 +35,7 @@
},
"theme": "dark",
"language": "en",
"sourceSha256": "ed33e779c76b7373c152ecf13e82064a09fe62608af1c75cea96c48369cfec11",
"sourceSha256": "ef5431f42e2c08425388d4d14df10de29040ea755ac64f81b734430729faae4f",
"imageSha256": "c33a7279165a4cec6fa6fadb6fd08cd967e082a17fe101ef442d27d36ae59b6b"
},
"room-contour-close": {
@@ -46,7 +46,7 @@
},
"theme": "dark",
"language": "en",
"sourceSha256": "ed33e779c76b7373c152ecf13e82064a09fe62608af1c75cea96c48369cfec11",
"sourceSha256": "ef5431f42e2c08425388d4d14df10de29040ea755ac64f81b734430729faae4f",
"imageSha256": "4d63670c7bcca33da21a6cf17275786bed12e8422a50d02d4dabe59645f2cd82"
},
"plan-context-tray": {
@@ -57,7 +57,7 @@
},
"theme": "dark",
"language": "en",
"sourceSha256": "ed33e779c76b7373c152ecf13e82064a09fe62608af1c75cea96c48369cfec11",
"sourceSha256": "ef5431f42e2c08425388d4d14df10de29040ea755ac64f81b734430729faae4f",
"imageSha256": "a6c526fede11bc3503fd2384bd4a3f6afa008f481c5c47578f06cae6c81c6f9b"
},
"device-editor": {
@@ -68,7 +68,7 @@
},
"theme": "dark",
"language": "en",
"sourceSha256": "ed33e779c76b7373c152ecf13e82064a09fe62608af1c75cea96c48369cfec11",
"sourceSha256": "ef5431f42e2c08425388d4d14df10de29040ea755ac64f81b734430729faae4f",
"imageSha256": "9585b59add4d35b5a6f028ce5b720ef1b77a3f8b19436d192cd1a0e6fc637264"
},
"device-display-preview": {
@@ -79,7 +79,7 @@
},
"theme": "dark",
"language": "en",
"sourceSha256": "ed33e779c76b7373c152ecf13e82064a09fe62608af1c75cea96c48369cfec11",
"sourceSha256": "ef5431f42e2c08425388d4d14df10de29040ea755ac64f81b734430729faae4f",
"imageSha256": "cfc317da4628d079a116ff71311fbf06b1eb3b181928a5ea7ba888ab936a5e8b"
},
"background-editor": {
@@ -90,7 +90,7 @@
},
"theme": "dark",
"language": "en",
"sourceSha256": "ed33e779c76b7373c152ecf13e82064a09fe62608af1c75cea96c48369cfec11",
"sourceSha256": "ef5431f42e2c08425388d4d14df10de29040ea755ac64f81b734430729faae4f",
"imageSha256": "d7cfe70551d9260169df8efd832e32ed7df99c7f60b55d1fd4a8322bd4c47175"
},
"room-card": {
@@ -101,7 +101,7 @@
},
"theme": "dark",
"language": "en",
"sourceSha256": "ed33e779c76b7373c152ecf13e82064a09fe62608af1c75cea96c48369cfec11",
"sourceSha256": "ef5431f42e2c08425388d4d14df10de29040ea755ac64f81b734430729faae4f",
"imageSha256": "029a3e69ec647a8a370d99e6bb7f9225833c526739076022f6b52ba54bff30ea"
},
"device-info": {
@@ -112,7 +112,7 @@
},
"theme": "dark",
"language": "en",
"sourceSha256": "ed33e779c76b7373c152ecf13e82064a09fe62608af1c75cea96c48369cfec11",
"sourceSha256": "ef5431f42e2c08425388d4d14df10de29040ea755ac64f81b734430729faae4f",
"imageSha256": "a06cbf83f09e2f67b3566d7c0b10e973db060c3d20786f26f74ada6a21937c3e"
}
}
+2 -2
View File
@@ -201,8 +201,8 @@ export const MUTANTS = [
+ 'reload, so the current session can still reproduce the geometry failure after Save',
patches: [{
file: 'src/houseplan-card.ts',
find: ' const candidate = canonicalizeConfigGeometry(this._serverCfg);',
replace: ' const candidate = this._serverCfg;',
find: ' const candidate = canonicalizeConfigGeometry(this._serverCfg);',
replace: ' const candidate = this._serverCfg;',
}],
},
{
+18 -13
View File
@@ -192,6 +192,7 @@ import {
canonicalizeLayoutGeometry,
canonicalizePosition,
} from './coordinate-canonicalization';
import { enqueueSerializedWrite } from './serialized-write-queue';
import { hasTranslation, langOf, t, type I18nKey } from './i18n';
import { CommandStack } from './command-stack';
import { resolvedSvgScreenBlend, svgScreenBlendSupported } from './glow-blend';
@@ -6947,21 +6948,25 @@ class HouseplanCard extends LitElement {
private _writeConfig(): Promise<void> {
this._writesPending++;
this._writeChain = this._writeChain
.catch(() => undefined) // a failed write must not poison the queue
.then(async () => {
if (!this._serverCfg) return;
this._dropLegacySegments();
const candidate = canonicalizeConfigGeometry(this._serverCfg);
const candidateFingerprint = contentFingerprint(candidate);
if (candidateFingerprint !== contentFingerprint(this._serverCfg)) this._cfgEpoch++;
this._writeChain = enqueueSerializedWrite(this._writeChain, async () => {
if (!this._serverCfg) return;
this._dropLegacySegments();
const candidate = canonicalizeConfigGeometry(this._serverCfg);
const candidateFingerprint = contentFingerprint(candidate);
// Do not replace the reactive root merely because the pure helper
// returned a clone. Besides an unnecessary render, that used to expose
// unrelated write-time cleanup as a visual change (#224 review H1).
// A real coordinate change still adopts the exact object sent below;
// willUpdate owns the corresponding geometry-epoch bump.
if (candidateFingerprint !== contentFingerprint(this._serverCfg)) {
this._serverCfg = candidate;
this._cfgContentFingerprint = candidateFingerprint;
const r = await this.hass.callWS({
type: 'houseplan/config/set', config: candidate, expected_rev: this._cfgRev,
});
this._cfgRev = r?.rev ?? this._cfgRev + 1;
}
this._cfgContentFingerprint = candidateFingerprint;
const r = await this.hass.callWS({
type: 'houseplan/config/set', config: candidate, expected_rev: this._cfgRev,
});
this._cfgRev = r?.rev ?? this._cfgRev + 1;
});
const mine = this._writeChain.finally(() => { this._writesPending--; });
// keep the chain itself unadorned so the next link waits for the write only
return mine;
+11
View File
@@ -0,0 +1,11 @@
/**
* Append one write to a promise chain without letting an earlier rejection
* poison later edits. The callback is deliberately invoked only when its turn
* starts, so it can read the latest local state at that moment.
*/
export function enqueueSerializedWrite(
previous: Promise<void>,
write: () => Promise<void>,
): Promise<void> {
return previous.catch(() => undefined).then(write);
}
@@ -54,10 +54,15 @@ test('one position changes only x/y and preserves future metadata (#224)', () =>
test('frontend write paths adopt canonical candidates before persistence (#224)', () => {
const source = readFileSync(new URL('../src/houseplan-card.ts', import.meta.url), 'utf8');
assert.match(source, /enqueueSerializedWrite\(this\._writeChain, async \(\) =>/);
assert.match(
source,
/const candidate = canonicalizeConfigGeometry\(this\._serverCfg\);[\s\S]*config: candidate/,
);
assert.match(
source,
/if \(candidateFingerprint !== contentFingerprint\(this\._serverCfg\)\) \{\s*this\._serverCfg = candidate;/,
);
assert.match(
source,
/const pos = canonicalizePosition\(this\._layout\[id\]\);[\s\S]*device_id: id, pos/,
+53
View File
@@ -0,0 +1,53 @@
import assert from 'node:assert/strict';
import test from 'node:test';
import { enqueueSerializedWrite } from '../test-build/serialized-write-queue.js';
function deferred() {
let resolve;
const promise = new Promise((done) => { resolve = done; });
return { promise, resolve };
}
test('a queued config write reads edits made while the previous write is in flight (#224)', async () => {
const firstStarted = deferred();
const releaseFirst = deferred();
let localConfig = 'first';
const observed = [];
let chain = Promise.resolve();
const enqueue = () => {
chain = enqueueSerializedWrite(chain, async () => {
observed.push(localConfig);
if (observed.length === 1) {
firstStarted.resolve();
await releaseFirst.promise;
}
});
return chain;
};
const first = enqueue();
await firstStarted.promise;
localConfig = 'second';
const second = enqueue();
localConfig = 'latest edit during await';
assert.deepEqual(observed, ['first'], 'the second write waits its turn');
releaseFirst.resolve();
await Promise.all([first, second]);
assert.deepEqual(observed, ['first', 'latest edit during await']);
});
test('a failed write does not poison the next queued edit (#224)', async () => {
const seen = [];
const failed = enqueueSerializedWrite(Promise.resolve(), async () => {
seen.push('failed');
throw new Error('offline');
});
const recovered = enqueueSerializedWrite(failed, async () => { seen.push('recovered'); });
await assert.rejects(failed, /offline/);
await recovered;
assert.deepEqual(seen, ['failed', 'recovered']);
});
+1
View File
@@ -26,6 +26,7 @@
"src/render-device-snapshot.ts",
"src/command-stack.ts",
"src/coordinate-canonicalization.ts",
"src/serialized-write-queue.ts",
"src/align-grid.ts",
"src/plan-optimizer.ts",
"src/furniture.ts",