From f2c2bc93339fedf201e521a87c74310a525453f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Louis=20Tr=C3=BCmpler?= <78563314+louistrue@users.noreply.github.com> Date: Sun, 23 Aug 2026 00:01:33 +0200 Subject: [PATCH] =?UTF-8?q?fix(viewer):=20main=20is=20red=20=E2=80=94=20th?= =?UTF-8?q?e=20frame=20test=20pinned=20the=20pre-#3039=20side=20normal?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `addElementMeshes.frame.test.ts` asserts the first side face's renderer z is `-1`. Since #3039 it is `+1`, and main has been red since both landed: not ok 2 - maps normals through the same frame change as positions error: 'side normal z (IFC +Y becomes renderer -Z): expected -1, got 1' **#3039 is right and this expectation was wrong.** Computed against the real corner order rather than re-derived by eye: column Width=1 Depth=2 Height=3, buildAxisBox box centre (0, 0, 1.5) face [0,4,5,1] all four corners at y = -1, centre (0, -1, 1.5) outward normal (0, -1, 0) IFC -> renderer (0, 0, +1) test expected (0, +1, 0) IFC -> renderer (0, 0, -1) That face is the -Y side of the box, so its outward normal points at -Y. The test pinned the INWARD direction, and it passed because the side normals WERE inward — which is the defect #3039 fixed by orienting each face away from the box centre. So this is the one case where editing an assertion is right: the behaviour genuinely changed and the assertion was pinned to the old, wrong behaviour. Changing an assertion demands proof the test still discriminates, so the mutation was run rather than argued: invert the orientation (normals inward again) 1 of 7 fails restored 7 of 7 pass addElementMeshes.test.ts (#3039's own suite) 20 of 20 pass The comment now states which face it is and why its normal points where it does, so the next reader does not have to re-derive the corner indices to know whether `1` or `-1` is correct. #3058 and #3039 were each green alone and red together. Neither is wrong by itself, so no amount of per-PR verification could have caught it: only a completed run on the merged result can, and main's Test queue was 18 commits behind when both landed. Recorded here because the merge cadence is what produced the blind spot, not either author. --- .../slices/addElementMeshes.frame.test.ts | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/apps/viewer/src/store/slices/addElementMeshes.frame.test.ts b/apps/viewer/src/store/slices/addElementMeshes.frame.test.ts index 303625e1b..eca209c20 100644 --- a/apps/viewer/src/store/slices/addElementMeshes.frame.test.ts +++ b/apps/viewer/src/store/slices/addElementMeshes.frame.test.ts @@ -107,12 +107,23 @@ describe('addElementMeshes: IFC storey-local to renderer frame (#2802)', () => { // The bottom face is [0, 0, -1] in IFC, so its x and y are BOTH zero and // it cannot see the `-face.normal[1]` term at all: flip that sign and the - // assertions above still pass. Vertices 8..11 are the first side face, - // IFC normal [0, 1, 0], which maps to renderer [0, 0, -1] and is the - // component that discriminates. + // assertions above still pass. Vertices 8..11 are the first side face, so + // that is the component which discriminates. + // + // That face is corners [0, 4, 5, 1], all four of which sit at y = -Depth/2, + // so it is the -Y face of the box and its OUTWARD IFC normal is [0, -1, 0], + // mapping to renderer [0, 0, +1]. + // + // This assertion previously read `-1`, pinning [0, +1, 0] — the INWARD + // direction. It passed because the side normals were inward at the time, + // which is the defect #3039 fixed by orienting each face away from the box + // centre. So the expectation was pinned to behaviour that was wrong, and + // #3058 and #3039 were each green alone and red together. Computed rather + // than re-derived by eye: box centre (0, 0, 1.5), face centre (0, -1, 1.5), + // difference (0, -1, 0). near(n[24], 0, 'side normal x'); near(n[25], 0, 'side normal y'); - near(n[26], -1, 'side normal z (IFC +Y becomes renderer -Z)'); + near(n[26], 1, 'side normal z (IFC -Y becomes renderer +Z)'); // Every normal is unit length, so none was left un-normalised by the swap. // Note this alone cannot see a sign flip, which is why the component