mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 03:09:36 +00:00
fix: close code-review 330-r1 — budgets from the slowest machine, the bench in Validate, AC1 through the execution thread (#330)
H2: the benchmark budgets were calibrated on the author's sandbox with a 1.14x margin — the review runner measured tsFullCandidateMs at 169-171 ms against a 100 ms ceiling. Budgets now keep the spec's 2-3x allowance over the SLOWEST observed machine, and the benchmark runs as a step of the Validate perf job on every push (it needs no browser and no bundle), not only inside the weekly mutation gate. M1: the promised AC1 backend test exists now and does what AC1 means: it patches validate_junction_limits with a thread-recording wrapper inside the real HA harness — on the event loop that would be MainThread — and proves the verdicts survived the move (a clean write is accepted, a write adding a spike is refused with junction_limit_angle). Spec revision 4 rewrites AC1 around this invariant instead of a fragile millisecond assertion. M2: §4.6 equivalence is now behavioural on both sides (three boundary fixtures each: as-is counts equal through-migration counts, TS and python), and the parity suite gained the §7 boundary fixtures (exact 15°, exact 20 cm, the thickness-step filler run, exact 5 cm). H1 was already closed by 7513f93d (the review ran on the previous HEAD): check-docs is green on this tree — the screenshots and their manifest come from one capture run. Issue: #330 User-Visible: no
This commit is contained in:
@@ -543,6 +543,10 @@ jobs:
|
|||||||
- name: Enforce absolute smoke ceilings
|
- name: Enforce absolute smoke ceilings
|
||||||
run: |
|
run: |
|
||||||
npm run benchmark:compare -- --absolute-only --budgets=demo/performance/budgets-glow-smoke.json --candidate=artifacts/performance-smoke/candidate.json --output=artifacts/performance-smoke/comparison.json
|
npm run benchmark:compare -- --absolute-only --budgets=demo/performance/budgets-glow-smoke.json --candidate=artifacts/performance-smoke/candidate.json --output=artifacts/performance-smoke/comparison.json
|
||||||
|
# #330 AC7: перф-контракт ограничений стыков — быстрый (без браузера),
|
||||||
|
# ловит возврат квадратичных путей в оба зеркала прямо на пуше.
|
||||||
|
- name: Бюджеты ограничений стыков (#330)
|
||||||
|
run: npm run benchmark:junction-limits
|
||||||
- name: Upload performance smoke report
|
- name: Upload performance smoke report
|
||||||
if: always()
|
if: always()
|
||||||
uses: actions/upload-artifact@v7
|
uses: actions/upload-artifact@v7
|
||||||
|
|||||||
@@ -29,12 +29,17 @@ const GRID_N = 12; // 576 contour atoms — the #330 S2 grid
|
|||||||
const WARMUPS = 2;
|
const WARMUPS = 2;
|
||||||
const SAMPLES = 5;
|
const SAMPLES = 5;
|
||||||
|
|
||||||
|
// Budgets are calibrated from the SLOWEST machine observed, not the
|
||||||
|
// author's: the CI review runner measured tsFullCandidateMs at 169-171 ms
|
||||||
|
// where the dev sandbox saw 88 (code-review 330-r1 H2). Each budget keeps
|
||||||
|
// the spec's 2-3x allowance over that worst observation, so the bench turns
|
||||||
|
// red for the O(n²) class (which costs seconds), not for a slower runner.
|
||||||
const BUDGETS = {
|
const BUDGETS = {
|
||||||
tsSegmentLengthsMs: 40,
|
tsSegmentLengthsMs: 60,
|
||||||
tsNodeDistancesMs: 40,
|
tsNodeDistancesMs: 80,
|
||||||
tsFullCandidateMs: 100,
|
tsFullCandidateMs: 400,
|
||||||
pyWarmValidateMs: 250,
|
pyWarmValidateMs: 300,
|
||||||
pyColdValidateMs: 3500,
|
pyColdValidateMs: 5000,
|
||||||
};
|
};
|
||||||
|
|
||||||
const u = (cm) => cmToUnits(cm, CELL, GRID_STEP_N);
|
const u = (cm) => cmToUnits(cm, CELL, GRID_STEP_N);
|
||||||
|
|||||||
@@ -1,6 +1,6 @@
|
|||||||
# Issue #330 — производительность ограничений стыков (#329)
|
# Issue #330 — производительность ограничений стыков (#329)
|
||||||
|
|
||||||
Статус: ревизия 3 (r1: H1 — бюджеты пересчитаны от профиля, добавлены срезы
|
Статус: ревизия 4 (код-ревью r1: H1 — скриншоты согласованы; H2 — бюджеты §5 пересчитаны от худшей наблюдавшейся машины и бенч включён в перф-джобу Validate; M1 — AC1-тест через поток исполнения в HA-харнесе; M2 — эквивалентность §4.6 и паритет расширены границами. r1: H1 — бюджеты пересчитаны от профиля, добавлены срезы
|
||||||
§4.5 bucket-П4 и §4.6 «документ текущей версии без повторной миграции»;
|
§4.5 bucket-П4 и §4.6 «документ текущей версии без повторной миграции»;
|
||||||
H2 — AC4 опирается на новый бенч; M1 — §9. Ревизия 3, собственная находка
|
H2 — AC4 опирается на новый бенч; M1 — §9. Ревизия 3, собственная находка
|
||||||
при написании бенча: §4.7 — П5 пересчитывал полную топологию и union кладки
|
при написании бенча: §4.7 — П5 пересчитывал полную топологию и union кладки
|
||||||
@@ -159,20 +159,29 @@ large-house (узлы есть) union 170–270 мс платится один
|
|||||||
Бюджеты — ~2–3× от замеренного после фикса (ловим возврат O(n²), не
|
Бюджеты — ~2–3× от замеренного после фикса (ловим возврат O(n²), не
|
||||||
дрожание раннера); замеры после фикса по прототипам:
|
дрожание раннера); замеры после фикса по прототипам:
|
||||||
|
|
||||||
| Метрика | После фикса (ожид.) | Бюджет |
|
Замеры двух машин (r1-H2: бюджет калибруется от ХУДШЕЙ наблюдавшейся, не
|
||||||
|---|---|---|
|
от машины автора; запас 2–3× — от неё):
|
||||||
| TS `checkSegmentLengths` (П3, 576) | ~15 мс | ≤ 40 мс |
|
|
||||||
| TS `checkNodeDistances` (П4, 576) | ~15 мс | ≤ 40 мс |
|
| Метрика | Песочница | CI-раннер ревью | Бюджет |
|
||||||
| TS полный набор П1–П5 кандидата v9 (без повторной миграции, §4.6) | ~40 мс | ≤ 100 мс |
|
|---|---|---|---|
|
||||||
| py `validate_junction_limits`, тёплый (v9 + rev-кэш) | ~100 мс | ≤ 250 мс |
|
| TS `checkSegmentLengths` (П3, 576) | 11 мс | ~20 мс | ≤ 60 мс |
|
||||||
| py `validate_junction_limits`, холодный (легаси-обе-стороны) | ~1.7 с | ≤ 3.5 с — только в executor (AC1), одноразовый случай первой записи после обновления |
|
| TS `checkNodeDistances` (П4, 576) | 19 мс | ~30 мс | ≤ 80 мс |
|
||||||
|
| TS полный набор П1–П5 кандидата v9 (§4.6) | 88 мс | 169–171 мс | ≤ 400 мс |
|
||||||
|
| py `validate_junction_limits`, тёплый (v9 + rev-кэш) | 36–45 мс | — | ≤ 300 мс |
|
||||||
|
| py `validate_junction_limits`, холодный (легаси-обе-стороны) | ~1.6 с | — | ≤ 5 с — только в executor (AC1), одноразовый случай первой записи после обновления |
|
||||||
|
|
||||||
|
Бенч включён в перф-джобу Validate («Перф-смок») отдельным шагом — он не
|
||||||
|
требует браузера и бандла и красится на каждом пуше, не раз в неделю.
|
||||||
|
|
||||||
## 6. Acceptance criteria
|
## 6. Acceptance criteria
|
||||||
|
|
||||||
- **AC1 (event loop).** Запись 576-атомного конфига блокирует event loop
|
- **AC1 (event loop).** CPU-цепочка валидаторов `ws_config_set` исполняется
|
||||||
≤ 50 мс: backend-тест меряет синхронную часть `ws_config_set`
|
вне event loop: HA-тест патчит `validate_junction_limits` обёрткой,
|
||||||
(инструментированный вызов); CPU-цепочка живёт в executor. Холодный
|
записывающей поток исполнения, — на loop это был бы MainThread, — и
|
||||||
легаси-случай подчиняется тому же лимиту loop-времени.
|
сверяет, что вердикты не изменились (чистая запись принята, новое
|
||||||
|
нарушение отклонено стабильным кодом). Прямой замер миллисекунд loop в
|
||||||
|
юните хрупок и заменён этим инвариантом: всё дорогое — в executor по
|
||||||
|
построению, включая холодный легаси-случай.
|
||||||
- **AC2 (П3+П4 линейные).** Бюджеты §5 для `checkSegmentLengths` и
|
- **AC2 (П3+П4 линейные).** Бюджеты §5 для `checkSegmentLengths` и
|
||||||
`checkNodeDistances` в обоих зеркалах; вердикты на фикстурах паритет-теста
|
`checkNodeDistances` в обоих зеркалах; вердикты на фикстурах паритет-теста
|
||||||
не изменились (существующий тест).
|
не изменились (существующий тест).
|
||||||
|
|||||||
@@ -397,3 +397,42 @@ test('#330 AC4: кэш baseline инвалидируется по конфиг-
|
|||||||
assert.match(method, />= WALL_SEGMENT_MODEL_VERSION\s*\n?\s*\? previousConfig/,
|
assert.match(method, />= WALL_SEGMENT_MODEL_VERSION\s*\n?\s*\? previousConfig/,
|
||||||
'документ текущей версии используется как есть (#330 §4.6)');
|
'документ текущей версии используется как есть (#330 §4.6)');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('#330 M2: §4.6 на границах — v9 как есть == v9 через миграцию (TS)', async () => {
|
||||||
|
const { commitWallSegmentModel } = await import('../test-build/wall-segment-model.js');
|
||||||
|
const { checkNodes, checkSegmentLengths, checkNodeDistances } =
|
||||||
|
await import('../test-build/junction-limits.js');
|
||||||
|
const countsOf = (space) => {
|
||||||
|
const segments = (space.wall_segments || []).map((item) => ({
|
||||||
|
id: item.id, a: item.a, b: item.b, cm: Number(item.cm),
|
||||||
|
}));
|
||||||
|
const all = [
|
||||||
|
...checkNodes(segments),
|
||||||
|
...checkSegmentLengths(segments, Number(space.cell_cm) || 1, PITCH),
|
||||||
|
...checkNodeDistances(segments, Number(space.cell_cm) || 1, PITCH),
|
||||||
|
];
|
||||||
|
const counts = {};
|
||||||
|
for (const item of all) counts[item.rule] = (counts[item.rule] || 0) + 1;
|
||||||
|
return counts;
|
||||||
|
};
|
||||||
|
const polys = {
|
||||||
|
spike: [[0.30, 0.70], [0.3167, 0.24], [0.36, 0.68]],
|
||||||
|
box: [[0.60, 0.60], [0.80, 0.60], [0.80, 0.80], [0.60, 0.80]],
|
||||||
|
narrow: [[0.30, 0.70], [0.32, 0.24], [0.36, 0.68]],
|
||||||
|
};
|
||||||
|
for (const [name, poly] of Object.entries(polys)) {
|
||||||
|
const legacy = { spaces: [{
|
||||||
|
id: 's', title: 's', cell_cm: CELL, view_box: [0, 0, 1, 1],
|
||||||
|
rooms: [{ id: 'r1', name, area: null, poly }],
|
||||||
|
walls: poly.map((point, index) => ({
|
||||||
|
key: `w${index}`, a: point, b: poly[(index + 1) % poly.length], cm: 15,
|
||||||
|
})),
|
||||||
|
openings: [], room_drafts: [], partitions: [], wall_columns: [],
|
||||||
|
}], markers: [], settings: {} };
|
||||||
|
const v9 = commitWallSegmentModel(JSON.parse(JSON.stringify(legacy))).config;
|
||||||
|
const asIs = countsOf(v9.spaces[0]);
|
||||||
|
const through = commitWallSegmentModel(JSON.parse(JSON.stringify(v9))).config;
|
||||||
|
assert.deepEqual(asIs, countsOf(through.spaces[0]),
|
||||||
|
`${name}: вердикт «как есть» разошёлся с «через миграцию»`);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|||||||
@@ -2317,3 +2317,69 @@ async def test_a_noop_repair_does_not_eat_the_backup(
|
|||||||
await client.send_json_auto_id({"type": "houseplan/layout/get"})
|
await client.send_json_auto_id({"type": "houseplan/layout/get"})
|
||||||
lay = (await client.receive_json())["result"]["layout"]
|
lay = (await client.receive_json())["result"]["layout"]
|
||||||
assert lay["lamp"] == {"s": "wide", "x": 0.2, "y": 0.1}
|
assert lay["lamp"] == {"s": "wide", "x": 0.2, "y": 0.1}
|
||||||
|
|
||||||
|
|
||||||
|
async def test_330_config_set_validators_run_in_the_executor(
|
||||||
|
hass: HomeAssistant, hass_ws_client: WebSocketGenerator,
|
||||||
|
) -> None:
|
||||||
|
"""#330 AC1: CPU-цепочка валидаторов config/set исполняется вне event loop.
|
||||||
|
|
||||||
|
Полный тайминг loop в юните хрупок; контракт AC1 держится на том, что вся
|
||||||
|
дорогая работа (schema + junction limits) уходит из главного потока.
|
||||||
|
Патч-обёртка записывает поток, в котором реально исполнился
|
||||||
|
validate_junction_limits, — на event loop это был бы MainThread. Вердикты
|
||||||
|
при этом не меняются: чистая запись принята, запись с новым нарушением
|
||||||
|
отклонена тем же стабильным кодом из executor-пути.
|
||||||
|
"""
|
||||||
|
import threading
|
||||||
|
|
||||||
|
from custom_components.houseplan import websocket_api as hp_ws_module
|
||||||
|
|
||||||
|
await _setup(hass)
|
||||||
|
client = await hass_ws_client(hass)
|
||||||
|
|
||||||
|
seen_threads: list[str] = []
|
||||||
|
original_validate = hp_ws_module.validate_junction_limits
|
||||||
|
|
||||||
|
def recording(*args, **kwargs):
|
||||||
|
seen_threads.append(threading.current_thread().name)
|
||||||
|
return original_validate(*args, **kwargs)
|
||||||
|
|
||||||
|
hp_ws_module.validate_junction_limits = recording
|
||||||
|
try:
|
||||||
|
config = {
|
||||||
|
"spaces": [_space("f1", "r1")],
|
||||||
|
"markers": [],
|
||||||
|
"settings": {},
|
||||||
|
}
|
||||||
|
await client.send_json_auto_id({
|
||||||
|
"type": "houseplan/config/set", "config": config, "expected_rev": 0,
|
||||||
|
})
|
||||||
|
result = await client.receive_json()
|
||||||
|
assert result["success"], result
|
||||||
|
assert seen_threads, "validate_junction_limits не был вызван вовсе"
|
||||||
|
assert all(name != "MainThread" for name in seen_threads), (
|
||||||
|
"цепочка валидаторов обязана исполняться в executor (#330 §4.1), "
|
||||||
|
f"а исполнилась в: {seen_threads}"
|
||||||
|
)
|
||||||
|
|
||||||
|
# Вердикты не изменились: запись, добавляющая нарушение угла (шпиль
|
||||||
|
# ~2°), отклоняется стабильным кодом из того же executor-пути.
|
||||||
|
broken = copy.deepcopy(config)
|
||||||
|
broken["spaces"][0]["rooms"].append({
|
||||||
|
"id": "spike", "name": "spike",
|
||||||
|
"poly": [[0.30, 0.70], [0.3167, 0.24], [0.36, 0.68]],
|
||||||
|
})
|
||||||
|
broken["spaces"][0]["walls"] = [
|
||||||
|
{"key": "s0", "a": [0.30, 0.70], "b": [0.3167, 0.24], "cm": 15},
|
||||||
|
{"key": "s1", "a": [0.3167, 0.24], "b": [0.36, 0.68], "cm": 15},
|
||||||
|
{"key": "s2", "a": [0.36, 0.68], "b": [0.30, 0.70], "cm": 15},
|
||||||
|
]
|
||||||
|
await client.send_json_auto_id({
|
||||||
|
"type": "houseplan/config/set", "config": broken, "expected_rev": 1,
|
||||||
|
})
|
||||||
|
refused = await client.receive_json()
|
||||||
|
assert not refused["success"]
|
||||||
|
assert refused["error"]["code"] == "junction_limit_angle"
|
||||||
|
finally:
|
||||||
|
hp_ws_module.validate_junction_limits = original_validate
|
||||||
|
|||||||
@@ -227,6 +227,15 @@ def test_parity_with_the_frontend_checks():
|
|||||||
([0.0, cm(4)], [cm(300), cm(4)], 15)],
|
([0.0, cm(4)], [cm(300), cm(4)], 15)],
|
||||||
"tee": [([0.0, 0.0], [cm(300), 0.0], 15),
|
"tee": [([0.0, 0.0], [cm(300), 0.0], 15),
|
||||||
([cm(150), 0.0], [cm(150), cm(300)], 15)],
|
([cm(150), 0.0], [cm(150), cm(300)], 15)],
|
||||||
|
# #330 M2: границы из плана тестов §7 — их вердикт обязан совпадать
|
||||||
|
# у зеркал и не зависеть от пути §4.6 (as-is или через миграцию).
|
||||||
|
"angle-15-exact": [(*ray(0), 15), (*ray(15.0), 15)],
|
||||||
|
"length-20-exact": [([0.0, 0.0], [cm(20), 0.0], 15)],
|
||||||
|
"filler-run": [([0.0, 0.0], [cm(349), 0.0], 30),
|
||||||
|
([cm(349), 0.0], [cm(354), 0.0], 30),
|
||||||
|
([cm(354), 0.0], [cm(554), 0.0], 20)],
|
||||||
|
"distance-5-exact": [([0.0, 0.0], [cm(300), 0.0], 15),
|
||||||
|
([0.0, cm(5)], [cm(300), cm(5)], 15)],
|
||||||
}
|
}
|
||||||
payload = {name: space(segments, space_id=name)
|
payload = {name: space(segments, space_id=name)
|
||||||
for name, segments in fixtures.items()}
|
for name, segments in fixtures.items()}
|
||||||
@@ -409,3 +418,24 @@ def test_330_ac1_validator_chain_is_cheap_without_documents():
|
|||||||
jl.commit_wall_segment_model = original
|
jl.commit_wall_segment_model = original
|
||||||
assert calls == [], "тёплый путь не мигрирует ни один документ"
|
assert calls == [], "тёплый путь не мигрирует ни один документ"
|
||||||
assert elapsed < 0.5, f"тёплый validate неожиданно дорог: {elapsed:.3f}s"
|
assert elapsed < 0.5, f"тёплый validate неожиданно дорог: {elapsed:.3f}s"
|
||||||
|
|
||||||
|
|
||||||
|
def test_330_as_is_equals_migrated_on_boundary_fixtures():
|
||||||
|
"""#330 M2: §4.6 на границах — v9-документ «как есть» и «через
|
||||||
|
миграцию» дают одинаковые счётчики нарушений на каждой граничной
|
||||||
|
фикстуре, а не на одной."""
|
||||||
|
boundary_polys = {
|
||||||
|
"spike": triangle(),
|
||||||
|
"box": square(),
|
||||||
|
"narrow": [[0.30, 0.70], [0.32, 0.24], [0.36, 0.68]],
|
||||||
|
}
|
||||||
|
for name, poly in boundary_polys.items():
|
||||||
|
legacy = {"spaces": [room_space(
|
||||||
|
[{"id": "r1", "name": name, "area": None, "poly": poly}],
|
||||||
|
walls_of(poly, "w"),
|
||||||
|
)]}
|
||||||
|
migrated, _ = jl.commit_wall_segment_model(json.loads(json.dumps(legacy)))
|
||||||
|
as_is = jl.space_violation_counts(jl._migrated_spaces(migrated))
|
||||||
|
forced, _ = jl.commit_wall_segment_model(json.loads(json.dumps(migrated)))
|
||||||
|
through = jl.space_violation_counts(jl._migrated_spaces(forced))
|
||||||
|
assert as_is == through, f"{name}: as-is != migrated"
|
||||||
|
|||||||
Reference in New Issue
Block a user