mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
fix: preflight diagnostics hash the judged candidate and the fallback dies with its dialog (#295)
CODE-REVIEW-295-r1, both Medium findings: M1 — _preflightDiagnostics hashed this._serverCfg, the saved config a space export would reproduce anyway. The hash now comes from the candidate the preflight actually judged: the report path passes r.config / d.config and the copy button reads the dialog's own candidate. The smoke no longer masks the difference — its dialog carries a candidate whose geometry differs from the saved config, and swapping the two must change the reported hash. M2 — the inline clipboard fallback survived dialog close and reopen, so a later refusal could hand the previous refusal's JSON to a bug report. The field now dies with its dialog: reset on open (_previewAlignDialog) and on every close path (escape, mode reset, successful apply, hp-close, cancel). Both regressions are pinned by execution: reverting either fix turns the extended smoke red (fingerprintTracksCandidate / fallbackClearedOnClose), and two new gate mutants keep it that way. Issue: #295 User-Visible: no
This commit is contained in:
File diff suppressed because one or more lines are too long
@@ -41,11 +41,15 @@ const out = await page.evaluate(async () => {
|
||||
&& logged.failures[0].reason === 'wall-exception' && logged.failures[0].detail === 'Error'
|
||||
&& typeof logged.preflightFingerprint === 'string' && typeof logged.checkedAt === 'string';
|
||||
|
||||
// Dialog: failed branch renders per-failure reasons
|
||||
// Dialog: failed branch renders per-failure reasons. The dialog carries the
|
||||
// CANDIDATE config the preflight judged — its geometry differs from the
|
||||
// saved one, exactly like a real refused optimize run (r.changed === true).
|
||||
const candidateCfg = structuredClone(card._serverCfg);
|
||||
candidateCfg.spaces[0].rooms[0].poly = [[0.1, 0.1], [0.6, 0.1], [0.6, 0.4], [0.1, 0.4]];
|
||||
card._alignDialog = {
|
||||
cm: 5, where: '', changed: true, busy: false, removeLiveMissingPositions: false,
|
||||
preflight,
|
||||
config: card._serverCfg, layout: {},
|
||||
config: candidateCfg, layout: {},
|
||||
report: { moved: 0, total: 0, rotated: 0, removedDrafts: 0, migrated: 0, canonicalized: 0,
|
||||
coordsCanonicalized: 0, latticeCoordinatesCanonicalized: 0, wallSegmentsMigrated: 0,
|
||||
wallsMerged: 0, spansMerged: 0, partitionsMerged: 0, partitionsReconciled: 0,
|
||||
@@ -80,6 +84,21 @@ const out = await page.evaluate(async () => {
|
||||
&& block.origin === 'runtime' && block.failures.length === 2
|
||||
&& block.failures[0].reason === 'wall-exception' && block.failures[1].detail === null;
|
||||
|
||||
// CODE-REVIEW-295-r1 M1: the geometry hash comes from the candidate the
|
||||
// preflight judged, not from the saved config — the block must carry what
|
||||
// a space export cannot reproduce.
|
||||
const candidateHash = block.failures[0].spaceGeometryFingerprint;
|
||||
const saved = card._alignDialog;
|
||||
card._alignDialog = { ...saved, config: card._serverCfg };
|
||||
copied = null;
|
||||
await card._copyPreflightDiagnostics(); await update();
|
||||
const savedHash = JSON.parse(copied).failures[0].spaceGeometryFingerprint;
|
||||
card._alignDialog = saved; await update();
|
||||
result.fingerprintTracksCandidate = typeof candidateHash === 'string'
|
||||
&& candidateHash.length > 0 && candidateHash !== savedHash;
|
||||
|
||||
Object.defineProperty(navigator, 'clipboard', { value: clipboard, configurable: true });
|
||||
|
||||
// Copy: clipboard failure -> inline fallback with the same JSON
|
||||
Object.defineProperty(navigator, 'clipboard', {
|
||||
value: { writeText: async () => { throw new Error('denied'); } }, configurable: true,
|
||||
@@ -88,6 +107,16 @@ const out = await page.evaluate(async () => {
|
||||
const pre = root().querySelector('hp-dialog details pre');
|
||||
result.inlineFallback = !!pre && pre.textContent.includes('houseplan-optimize-preflight');
|
||||
|
||||
// CODE-REVIEW-295-r1 M2: the inline fallback belongs to one dialog showing.
|
||||
// Closing through the real hp-close path clears it, so a later refusal can
|
||||
// never exhibit the previous refusal's JSON.
|
||||
root().querySelector('hp-dialog')?.dispatchEvent(new CustomEvent(
|
||||
'hp-close', { bubbles: true, composed: true },
|
||||
));
|
||||
await update();
|
||||
result.fallbackClearedOnClose = card._alignDialog === null
|
||||
&& card._preflightClipboardFallback === null;
|
||||
|
||||
console.warn = origWarn;
|
||||
return result;
|
||||
});
|
||||
|
||||
Vendored
+6
-6
File diff suppressed because one or more lines are too long
@@ -2,7 +2,7 @@
|
||||
"version": 1,
|
||||
"fixture": "synthetic-only",
|
||||
"chromium": "151.0.7922.34",
|
||||
"sourceFingerprint": "2bd6abd8fa9674748b19b3ae12d65e093d804d4d9e4f0294e3fbdd6773effaab",
|
||||
"sourceFingerprint": "b89040573ef8a8f6e1088d115b634f50ed7169240c5cf7e745d6287fce83980e",
|
||||
"captureScriptSha256": "ce2e9542fed9dade3085be87d16f69adb2ac8262893ad78ad966b1b9673f2983",
|
||||
"command": "npm run build && node demo/docs/capture.mjs",
|
||||
"scenarios": {
|
||||
@@ -14,7 +14,7 @@
|
||||
},
|
||||
"theme": "dark",
|
||||
"language": "en",
|
||||
"sourceSha256": "2bd6abd8fa9674748b19b3ae12d65e093d804d4d9e4f0294e3fbdd6773effaab",
|
||||
"sourceSha256": "b89040573ef8a8f6e1088d115b634f50ed7169240c5cf7e745d6287fce83980e",
|
||||
"imageSha256": "36223106c073f07d8cc3ecf8eaab37192ebb2687daba65c5c21047d0b7890de0"
|
||||
},
|
||||
"view-touch": {
|
||||
@@ -25,7 +25,7 @@
|
||||
},
|
||||
"theme": "dark",
|
||||
"language": "en",
|
||||
"sourceSha256": "2bd6abd8fa9674748b19b3ae12d65e093d804d4d9e4f0294e3fbdd6773effaab",
|
||||
"sourceSha256": "b89040573ef8a8f6e1088d115b634f50ed7169240c5cf7e745d6287fce83980e",
|
||||
"imageSha256": "41e3ba67f8db0e98f26f484293af83ef937c369ca5ca6a59a3350d8954c906f4"
|
||||
},
|
||||
"space-create": {
|
||||
@@ -36,7 +36,7 @@
|
||||
},
|
||||
"theme": "dark",
|
||||
"language": "en",
|
||||
"sourceSha256": "2bd6abd8fa9674748b19b3ae12d65e093d804d4d9e4f0294e3fbdd6773effaab",
|
||||
"sourceSha256": "b89040573ef8a8f6e1088d115b634f50ed7169240c5cf7e745d6287fce83980e",
|
||||
"imageSha256": "c33a7279165a4cec6fa6fadb6fd08cd967e082a17fe101ef442d27d36ae59b6b"
|
||||
},
|
||||
"room-contour-close": {
|
||||
@@ -47,7 +47,7 @@
|
||||
},
|
||||
"theme": "dark",
|
||||
"language": "en",
|
||||
"sourceSha256": "2bd6abd8fa9674748b19b3ae12d65e093d804d4d9e4f0294e3fbdd6773effaab",
|
||||
"sourceSha256": "b89040573ef8a8f6e1088d115b634f50ed7169240c5cf7e745d6287fce83980e",
|
||||
"imageSha256": "b4777162eae89e0d95801721330bcd74ba761624b82b3362069d7e2d38317e08"
|
||||
},
|
||||
"plan-context-tray": {
|
||||
@@ -58,7 +58,7 @@
|
||||
},
|
||||
"theme": "dark",
|
||||
"language": "en",
|
||||
"sourceSha256": "2bd6abd8fa9674748b19b3ae12d65e093d804d4d9e4f0294e3fbdd6773effaab",
|
||||
"sourceSha256": "b89040573ef8a8f6e1088d115b634f50ed7169240c5cf7e745d6287fce83980e",
|
||||
"imageSha256": "c0e28edf82f45ccc6df568d3023b681e9e4394262ad34c1605fe9eacdb57a390"
|
||||
},
|
||||
"device-editor": {
|
||||
@@ -69,7 +69,7 @@
|
||||
},
|
||||
"theme": "dark",
|
||||
"language": "en",
|
||||
"sourceSha256": "2bd6abd8fa9674748b19b3ae12d65e093d804d4d9e4f0294e3fbdd6773effaab",
|
||||
"sourceSha256": "b89040573ef8a8f6e1088d115b634f50ed7169240c5cf7e745d6287fce83980e",
|
||||
"imageSha256": "d0ffd31ce80bfde21ab75da356a5fc1af38246f2b301030880320620c228d89d"
|
||||
},
|
||||
"device-display-preview": {
|
||||
@@ -80,7 +80,7 @@
|
||||
},
|
||||
"theme": "dark",
|
||||
"language": "en",
|
||||
"sourceSha256": "2bd6abd8fa9674748b19b3ae12d65e093d804d4d9e4f0294e3fbdd6773effaab",
|
||||
"sourceSha256": "b89040573ef8a8f6e1088d115b634f50ed7169240c5cf7e745d6287fce83980e",
|
||||
"imageSha256": "2cdabae1f89c3286e4fac0ce30f757ee1690b707ab8a5488748b7cd420626160"
|
||||
},
|
||||
"background-editor": {
|
||||
@@ -91,7 +91,7 @@
|
||||
},
|
||||
"theme": "dark",
|
||||
"language": "en",
|
||||
"sourceSha256": "2bd6abd8fa9674748b19b3ae12d65e093d804d4d9e4f0294e3fbdd6773effaab",
|
||||
"sourceSha256": "b89040573ef8a8f6e1088d115b634f50ed7169240c5cf7e745d6287fce83980e",
|
||||
"imageSha256": "30147bb00a90eea7136b4cee30995f6e6a9217b5132f3e8d3ad7471413b1af8a"
|
||||
},
|
||||
"room-card": {
|
||||
@@ -102,7 +102,7 @@
|
||||
},
|
||||
"theme": "dark",
|
||||
"language": "en",
|
||||
"sourceSha256": "2bd6abd8fa9674748b19b3ae12d65e093d804d4d9e4f0294e3fbdd6773effaab",
|
||||
"sourceSha256": "b89040573ef8a8f6e1088d115b634f50ed7169240c5cf7e745d6287fce83980e",
|
||||
"imageSha256": "029a3e69ec647a8a370d99e6bb7f9225833c526739076022f6b52ba54bff30ea"
|
||||
},
|
||||
"device-info": {
|
||||
@@ -113,7 +113,7 @@
|
||||
},
|
||||
"theme": "dark",
|
||||
"language": "en",
|
||||
"sourceSha256": "2bd6abd8fa9674748b19b3ae12d65e093d804d4d9e4f0294e3fbdd6773effaab",
|
||||
"sourceSha256": "b89040573ef8a8f6e1088d115b634f50ed7169240c5cf7e745d6287fce83980e",
|
||||
"imageSha256": "dd492f53150b7149085daada5cce9eeae9bde9e7ea1d86679a54b3041f72f517"
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2258,6 +2258,33 @@ export const MUTANTS = [
|
||||
replace: " ${failure.displayName}",
|
||||
}],
|
||||
},
|
||||
{
|
||||
id: 'preflight-fingerprint-from-saved-config',
|
||||
// CODE-REVIEW-295-r1 M1: хэш геометрии обязан браться из кандидата,
|
||||
// который проверял preflight, а не из сохранённого конфига — иначе блок
|
||||
// повторяет то, что и так даст экспорт пространства.
|
||||
guard: 'npx tsc -p tsconfig.test.json && node scripts/fix-test-build.mjs '
|
||||
+ '&& node demo/smoke_preflight_diagnostics.mjs',
|
||||
because: 'диагностика с хэшем сохранённой геометрии не несёт ничего сверх экспорта (AC4)',
|
||||
patches: [{
|
||||
file: 'src/houseplan-card.ts',
|
||||
find: " const spacesById = new Map(((candidate as any)?.spaces || [])",
|
||||
replace: " const spacesById = new Map(((this._serverCfg as any)?.spaces || [])",
|
||||
}],
|
||||
},
|
||||
{
|
||||
id: 'preflight-fallback-survives-dialog-close',
|
||||
// CODE-REVIEW-295-r1 M2: инлайн-фолбэк живёт одно показание диалога;
|
||||
// переживший закрытие блок подсунет в отчёт диагностику чужого отказа.
|
||||
guard: 'npx tsc -p tsconfig.test.json && node scripts/fix-test-build.mjs '
|
||||
+ '&& node demo/smoke_preflight_diagnostics.mjs',
|
||||
because: 'застрявший фолбэк отдаёт в баг-репорт JSON предыдущего отказа, не текущего (AC4)',
|
||||
patches: [{
|
||||
file: 'src/houseplan-card.ts',
|
||||
find: "dismiss-on-scrim @hp-close=${() => { this._alignDialog = null; this._preflightClipboardFallback = null; }}>",
|
||||
replace: "dismiss-on-scrim @hp-close=${() => { this._alignDialog = null; }}>",
|
||||
}],
|
||||
},
|
||||
{
|
||||
id: 'preflight-diagnostics-without-reason',
|
||||
// #295: копируемый блок без reason бесполезен для отчёта об ошибке.
|
||||
@@ -2278,8 +2305,8 @@ export const MUTANTS = [
|
||||
because: 'структурированная запись отказа в консоли — часть контракта диагностики #295',
|
||||
patches: [{
|
||||
file: 'src/houseplan-card.ts',
|
||||
find: " console.warn('[houseplan] optimize preflight failed', this._preflightDiagnostics(preflight));",
|
||||
replace: " void preflight;",
|
||||
find: " console.warn('[houseplan] optimize preflight failed', this._preflightDiagnostics(preflight, candidate));",
|
||||
replace: " void preflight; void candidate;",
|
||||
}],
|
||||
},
|
||||
{
|
||||
|
||||
+28
-10
@@ -2549,7 +2549,7 @@ class HouseplanCard extends LitElement {
|
||||
if (this._openingInfo) { this._openingInfo = null; return; }
|
||||
if (this._infoCard) { this._closeInfoCard(); return; }
|
||||
if (this._rulesDialog) { this._rulesDialog = null; return; }
|
||||
if (this._alignDialog) { this._alignDialog = null; return; }
|
||||
if (this._alignDialog) { this._alignDialog = null; this._preflightClipboardFallback = null; return; }
|
||||
if (this._backupImportDialog) { this._backupImportDialog = null; return; }
|
||||
if (this._backupExportDialog) { this._backupExportDialog = null; return; }
|
||||
if (this._settingsDialog) { this._settingsDialog = null; return; }
|
||||
@@ -6481,6 +6481,7 @@ class HouseplanCard extends LitElement {
|
||||
this._closeInfoCard();
|
||||
this._rulesDialog = null;
|
||||
this._alignDialog = null;
|
||||
this._preflightClipboardFallback = null;
|
||||
this._backupImportDialog = null;
|
||||
this._backupExportDialog = null;
|
||||
this._settingsDialog = null;
|
||||
@@ -15841,8 +15842,15 @@ class HouseplanCard extends LitElement {
|
||||
* refusal depends on live card state and a saved export may not reproduce
|
||||
* it, so the payload carries what the export cannot.
|
||||
*/
|
||||
private _preflightDiagnostics(preflight: OptimizeGeometryPreflightResult): object {
|
||||
const spacesById = new Map((this._serverCfg?.spaces || [])
|
||||
private _preflightDiagnostics(
|
||||
preflight: OptimizeGeometryPreflightResult,
|
||||
candidate: ServerConfig | null,
|
||||
): object {
|
||||
// CODE-REVIEW-295-r1 M1: hash the CANDIDATE spaces the preflight judged,
|
||||
// not the already-saved config — a saved-config hash is exactly what a
|
||||
// space export would reproduce, and the block promises what the export
|
||||
// does not carry.
|
||||
const spacesById = new Map(((candidate as any)?.spaces || [])
|
||||
.map((space: any) => [String(space?.id || ''), space]));
|
||||
return {
|
||||
kind: 'houseplan-optimize-preflight',
|
||||
@@ -15864,11 +15872,14 @@ class HouseplanCard extends LitElement {
|
||||
|
||||
/** #295: dev-log once per distinct failing preflight, not once per render. */
|
||||
private _reportedPreflightFingerprint: string | null = null;
|
||||
private _reportPreflightFailure(preflight: OptimizeGeometryPreflightResult): void {
|
||||
private _reportPreflightFailure(
|
||||
preflight: OptimizeGeometryPreflightResult,
|
||||
candidate: ServerConfig | null,
|
||||
): void {
|
||||
if (preflight.ok || preflight.fingerprint === this._reportedPreflightFingerprint) return;
|
||||
this._reportedPreflightFingerprint = preflight.fingerprint;
|
||||
// eslint-disable-next-line no-console
|
||||
console.warn('[houseplan] optimize preflight failed', this._preflightDiagnostics(preflight));
|
||||
console.warn('[houseplan] optimize preflight failed', this._preflightDiagnostics(preflight, candidate));
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -15887,7 +15898,9 @@ class HouseplanCard extends LitElement {
|
||||
private async _copyPreflightDiagnostics(): Promise<void> {
|
||||
const preflight = this._alignDialog?.preflight;
|
||||
if (!preflight || preflight.ok) return;
|
||||
const text = JSON.stringify(this._preflightDiagnostics(preflight), null, 2);
|
||||
const text = JSON.stringify(
|
||||
this._preflightDiagnostics(preflight, this._alignDialog?.config ?? null), null, 2,
|
||||
);
|
||||
try {
|
||||
await navigator.clipboard.writeText(text);
|
||||
this._preflightClipboardFallback = null;
|
||||
@@ -15991,7 +16004,7 @@ class HouseplanCard extends LitElement {
|
||||
return;
|
||||
}
|
||||
const preflight = r.changed ? this._checkOptimizeGeometry(r.config) : null;
|
||||
if (preflight) this._reportPreflightFailure(preflight);
|
||||
if (preflight) this._reportPreflightFailure(preflight, r.config);
|
||||
// The maximum geometry shift is an UPPER BOUND, not a sample. The run
|
||||
// measured every element in the centimetres of ITS OWN space — converting
|
||||
// one normalised maximum through the first space's `cell_cm` understated
|
||||
@@ -16000,6 +16013,10 @@ class HouseplanCard extends LitElement {
|
||||
const cm = Math.ceil(r.report.maxShiftCm * 10) / 10;
|
||||
const sp = spaces.find((x: any) => x?.id != null && String(x.id) === r.report.maxSpace);
|
||||
const where = spaces.length > 1 && sp ? String(sp.title || sp.id) : '';
|
||||
// CODE-REVIEW-295-r1 M2: the inline clipboard fallback belongs to one
|
||||
// dialog showing — a reopened dialog must not display the previous
|
||||
// refusal's JSON while the visible reasons already describe a new one.
|
||||
this._preflightClipboardFallback = null;
|
||||
this._alignDialog = {
|
||||
report: r.report, config: r.config, layout: r.layout, cm, where,
|
||||
preflight, changed: r.changed, busy: false, removeLiveMissingPositions,
|
||||
@@ -16024,7 +16041,7 @@ class HouseplanCard extends LitElement {
|
||||
const fingerprint = contentFingerprint(d.config);
|
||||
if (d.preflight.fingerprint !== fingerprint) {
|
||||
const preflight = this._checkOptimizeGeometry(d.config);
|
||||
this._reportPreflightFailure(preflight);
|
||||
this._reportPreflightFailure(preflight, d.config);
|
||||
d = { ...d, preflight };
|
||||
this._alignDialog = d;
|
||||
if (!preflight.ok) return;
|
||||
@@ -16057,6 +16074,7 @@ class HouseplanCard extends LitElement {
|
||||
this._maybeRebuildDevices();
|
||||
this._cacheSnapshot();
|
||||
this._alignDialog = null;
|
||||
this._preflightClipboardFallback = null;
|
||||
this.requestUpdate();
|
||||
this._showToast(this._t('gs.align_done', {
|
||||
n: String(d.report.moved),
|
||||
@@ -17161,7 +17179,7 @@ class HouseplanCard extends LitElement {
|
||||
const visibleDetails = referenceDetails.slice(0, 10);
|
||||
const remainingDetails = Math.max(0, referenceDetails.length - visibleDetails.length);
|
||||
return html`<hp-dialog .hass=${this.hass} .title=${this._t('gs.align_title')} icon="mdi:broom"
|
||||
dismiss-on-scrim @hp-close=${() => (this._alignDialog = null)}>
|
||||
dismiss-on-scrim @hp-close=${() => { this._alignDialog = null; this._preflightClipboardFallback = null; }}>
|
||||
<div class="body">
|
||||
${failed
|
||||
? html`
|
||||
@@ -17324,7 +17342,7 @@ class HouseplanCard extends LitElement {
|
||||
</div>
|
||||
<div class="row" slot="footer">
|
||||
<span class="spacer"></span>
|
||||
<button class="btn ghost" @click=${() => (this._alignDialog = null)}>${this._t('btn.cancel')}</button>
|
||||
<button class="btn ghost" @click=${() => { this._alignDialog = null; this._preflightClipboardFallback = null; }}>${this._t('btn.cancel')}</button>
|
||||
${!d.changed || !d.preflight?.ok ? nothing : html`
|
||||
<button class="btn on" @click=${this._runAlignToGrid} ?disabled=${d.busy}>
|
||||
<ha-icon icon="mdi:check"></ha-icon>${d.busy ? '…' : this._t('gs.align_run')}
|
||||
|
||||
Reference in New Issue
Block a user