Repository navigation
Address structural rendering, loading, and ephemeris weaknesses - #135
Protonmatter wants to merge 15 commits into
Conversation
Protonmatter
left a comment
Protonmatter
left a comment
There was a problem hiding this comment.
Review of f66dc19
Summary: this is a substantial improvement, and the ephemeris math checks out. CI is red for one fixable reason, and a local build shows one user-visible regression (JPEG reference images rejected; inline comment on referenceImageLoader.js). I'd keep it as a draft until both are fixed.
How this was reviewed: I built the PR head locally (build_wasm.py + build_web.py) and loaded it in headless Chromium with software WebGL at 1440×900. I compared it against the same build of 02aaec8 and read the diff by area. Frame-time numbers come from software GL and are not reported.
Blocking (not on changed lines, so here)
1. CI: tools/browser_smoke.py was not updated for moon demand loading.
- Cause:
enterOrreryInnernow callssyncCatalogueDemand(true), and on the default Sun overview no moon parent reaches 32 px, somoonelements.jsnever loads.MOON_READINESS_JS(lines 32-36, 95-120) still requires all 21 moons to have elements. - Effect: readiness times out at 45 s with
data-smoke-moon-knots="0". JavaScript coverage fails inbrowser_smoke.py, and Release gate then fails withmissing/ambiguous/oversized coverage report: combined/coverage-summary.json. - Minimal fix: after the lifecycle loop, select a satellite system through the real control. Loaded elements persist in the shared
MOONS, so later phases still work:
const a=document.getElementById("orreryAnchor"); a.value="Saturn"; a.dispatchEvent(new Event("change",{bubbles:true})); - Optional: fail fast on
state.catalogueStatus?.moons==="failed"rather than waiting out 45 s.
2. CI, hidden behind item 1: smoke phases assume a synchronous paint().
- Cause: the alias and reset phases (
browser_smoke.py:223-246) reado.moonsAliasedCountand the accuracy text immediately after dispatchinginput.paint()now only queues a frame (orrery.js:1979). - Effect: once item 1 is fixed, these phases will see stale values.
- Fix: await one animation frame (or poll), or expose a test-only draw-now hook.
3. JPEG reference images are rejected. See the inline comment on referenceImageLoader.js.
Should fix before merge
Inline comments on:
renderQuality.js: the tier controller oscillates, and can stay degraded forever.orrery.js:3664: hover invalidates frames.labelLayout.js: the selected body can lose its label.
Needs a decision
Inline comment on ephemeris-snapshot-v3.schema.json: additive fields under the same v3 identifier.
Minor
Inline comments on:
labelPresenter.js: per-frame layout reads remain.orrery.js:3745: smaller touch targets.sky.js: twilight hit targets and daytime star trajectories.bundle_web.mjs: source-map URLs.wasm_provenance.py: receipt profile.
PR description: it calls the rotation coefficients "spectroscopic/Doppler-like". The code and structural-science-dispositions.md correctly say Doppler-feature tracking, explicitly not the spectroscopic fit.
Verified correct
-
Phase reuse: bit-identical. A per-call
Vecis built from that call's ownt, nothing is reused across epochs or bodies, and the only shared state is a read-onlyOnceLock.cargo test -p solar-ephemeris --release --lockedpasses (89 lib + 4). -
Speed constant: now
AU_KM/(365.25*86400)= 4.7404705; 4.74057 is gone from Rust, JS and Python. -
Future dates: UT1 is now modeled past the EOP end with null uncertainty, so this is a real fix for Earth rotation. TT still holds UTC+69.184 s, which is now a declared assumption.
-
Rotation attribution: comment-only change; the numbers are unchanged and consistent across Rust, JS, Python and
MATH.md. -
Independent evidence: against Horizons DE441 with manifest and response hashes. Validation fails closed; 141/160 pass, and
--require-all-passexits 1. -
wasm-releaseprofile: inheritsreleasewithpanic=abort, andbuild_wasm.pyuses it consistently. -
Server dates: unsupported dates fail before coordinator, cache or upstream work.
-
Frame scheduler: one pending frame, re-arm on invalidate-during-draw, generation guards, and no draw after leave, hide or context loss.
-
Texture lifetime: bitmaps are closed on every abort, fail and cancel path, with at most one upload per frame.
-
WASM broker: a compiled module is shared while each worker has its own instance and memory; rejected compiles are evicted, so Retry works.
-
Bundling:
- Deterministic: two runs were byte-identical with no absolute paths.
- Pinned: esbuild 0.28.2 with sha512 integrity.
- No duplicated modules: the four previously double-loaded ones (
orreryMath,orreryShaders,orreryTime,visualAssets) each now compile into exactly one chunk.
-
Sky: constellations are precessed with the engine's J2000-to-true-equator matrix, applied once. Daytime stars now fade out while the accessible list keeps them.
-
Measured against 02aaec8:
02aaec8 f66dc19 Solar System open: requests 108 44 Solar System open: transfer 8.4 MB 4.0 MB Stray top-left labels yes gone Saturn focus now frames the planet.
Not addressed (noting only)
- East-on-right compass convention (kept by design).
- Geometric dome versus refracted rise/set (now disclosed).
- The 96 MB
sol-source.tar.gzinside the site. - The 200 ms visible-response target (the PR states this).
Generated by Claude Code
| // Admitted source-qualified references are PNG. Read IHDR before resizing so | ||
| // a requested display grid cannot disguise drift in the pinned source grid. | ||
| const view=new DataView(bytes),signature=[137,80,78,71,13,10,26,10]; | ||
| if(bytes.byteLength<24 || !signature.every((b,i)=>view.getUint8(i)===b) |
There was a problem hiding this comment.
Blocking: regression. This loader accepts PNG only, but three reference assets in visual-assets.v1.json that now route through it (orrery.js:739) are JPEG:
textures/reference/earth-land-2004.jpgtextures/reference/earth-lights-2016.jpgtextures/reference/moon-reference.jpg
They always throw Reference source grid mismatch, are marked unavailable, and are never retried. In a local build of f66dc19, selecting Earth logs Reference image unavailable for both Earth files, and the Earth caption loses its night-lights reference line. The base build (02aaec8) loaded them through new Image().
Suggested fix: check the format the manifest declares (or sniff the JPEG SOI FF D8 FF plus the SOF0/SOF2 dimensions), and pass the matching Blob type. Add one test that uses a JPEG asset from the real manifest.
Generated by Claude Code
| if(overload===8&&tier>0&×tamp-lastChange>=1500){ | ||
| tier--;lastChange=timestamp;resetEvidence();publish('Sustained frame work or delivery delay.');return true; | ||
| } | ||
| if(underload===180&&tier<2&×tamp-lastChange>=6000){ | ||
| tier++;lastChange=timestamp;resetEvidence();publish('Sustained frame headroom.');return true; |
There was a problem hiding this comment.
Two problems with the tier controller.
-
It oscillates indefinitely. Nothing remembers that a higher tier just failed. A simulated GPU needing about 20 ms per frame at High drops to Balanced at 0.3 s, goes back up at 6.3 s, drops again at 7.8 s, and repeats every 7.5 s (19 changes in the run). Each change resizes the drawing buffer and reallocates targets, so the scene visibly jumps.
- Fix: back off after a failed upgrade (for example, double the 6 s hold each time), or block re-entry to a tier that just failed.
-
It can stay degraded forever. Upgrades require
continuoussamples (line 33), but downgrades also count non-continuous drag frames (frameMs>24).frameMsis JS time, which a lower render scale does not reduce. With Animate off (the reduced-motion default), a few slow drags drop the tier and nothing brings it back; the simulation stayed at Balanced through 100k fast frames.lastChange=-Infinityalso lets startup jank cause the first drop with no cooldown.- Fix: allow upgrades from fast non-continuous samples, ignore an initial window, and base downgrades on frame interval rather than JS time.
Generated by Claude Code
| frameScheduler.invalidate({input:true}); if (!moved) pick(e); drop(e); }); | ||
| canvas.addEventListener("pointercancel", drop); | ||
| canvas.addEventListener("pointermove", (e) => { | ||
| frameScheduler.invalidate({input:true}); |
There was a problem hiding this comment.
This invalidate({input:true}) runs before the if (!pointers.has(e.pointerId)) return; check on the next line. As a result, any mouse movement over the canvas schedules a full scene draw plus label layout, even with Animate off and no drag in progress. That keeps an idle scene rendering at display rate while hovering, and it also feeds slow non-continuous samples into renderQuality (see the comment there).
Suggested fix: move the invalidate below the early return.
Generated by Claude Code
| const phys = BODY[name]; | ||
| const disc = projectOpaqueDisc({id:name,position:p, | ||
| radius:displayRadiusAU(name)*Math.min(1,phys.polarKm/phys.radiusKm)},vp,{width:w,height:h}); | ||
| const hitRadius = Math.max(12, disc?.radius || 0); |
There was a problem hiding this comment.
Minor (plausible, not device-tested): the planet hit tolerance drops from 34 px to max(12, disc), and moons from 18 px to 12 px (line 3762). In the enlarged overview on a phone, Mercury, Venus and Earth near the Sun become hard to tap.
Consider a floor of about 22 px when e.pointerType === 'touch'.
Generated by Claude Code
| } | ||
| } | ||
| } | ||
| const box = positions.map(([x,y])=>({id:c.id,x,y,width:w,height:h,callout:false})).find(fits); |
There was a problem hiding this comment.
With the callout fallback removed, a priority-0 label (the selected body) is silently dropped when all four candidate positions collide or touch an edge. Example: select Venus while Mercury, Venus and Earth overlap near the Sun in the overview, and the selected body has no name on screen.
Suggested fix: keep a fallback placement for priority 0 (or ≤ 2), for example a leader-line callout or the nearest free slot. Alternatively, guarantee placement by evicting a lower-priority label.
Generated by Claude Code
| for (let i = 0; i < labelEls.length; i++) { | ||
| const el = labelEls[i], it = items[i]; | ||
| delete el.dataset.projectionX; delete el.dataset.projectionY; | ||
| el.style.display = it ? "block" : "none"; |
There was a problem hiding this comment.
Minor perf: per-frame forced layout remains here.
- This pass sets
display:blockon every label, including labels hidden in the previous frame. jumps.getBoundingClientRect()andhost.getBoundingClientRect()are then read every frame, anddataset.projectionX/Yis deleted and re-set every frame.
Whenever any label was suppressed last frame (the normal case), each frame does a write, then a forced synchronous layout, then another write. While animating, the accuracy text is rewritten just before the clientWidth read, which adds a second layout.
Suggested fix: cache topInset (invalidate via ResizeObserver or when the jump controls change), and only toggle display once the final placement is known.
Generated by Claude Code
| ctx.globalAlpha = starAlpha; | ||
| for (const b of snap.bodies || []) { | ||
| if (BODY_STYLE[b.name] || !(b.alt_deg > 0)) continue; // BODY_STYLE = Sun/Moon/planets | ||
| if (!starAlpha || BODY_STYLE[b.name] || !(b.alt_deg > 0)) continue; // BODY_STYLE = Sun/Moon/planets |
There was a problem hiding this comment.
Low: daylight suppression only drops stars from plotted (line 382) when starAlpha is exactly 0.
- Clickable while nearly invisible. With the Sun between roughly -3° and 0°, stars draw at 0 to 0.17 opacity yet still take hover and click and show tooltips. The governing plan says daylight suppresses star hit targets too.
- Daytime trajectories. The trajectory for a selected catalogue star (line 394) still draws at full strength when the star itself is hidden. For example, picking Vega from the accessible list at midday draws a path with no marker.
Suggested fix: only push stars to plotted above a visibility threshold (for example starAlpha >= 0.25), and apply starAlpha to star trajectories, or skip them when it is 0.
Generated by Claude Code
| } | ||
| } | ||
| }, | ||
| "star_frame": { |
There was a problem hiding this comment.
Needs a decision: the contract was widened without a version change.
The PR adds several things while schema_version stays ephemeris-snapshot.v3:
- root
star_frame catalogue_ra_deg/catalogue_dec_degon bodies- new enum values
future_nominal_utcandmodeled_future - a nullable
dut1_uncertainty_seconds dut1_secondswith its bounds removed
The root and body objects are additionalProperties: false, so a reader validating against the previously published v3 schema rejects every new on-device snapshot (star_frame is always emitted, lib.rs:773/844) and every future-dated one. New readers still accept old snapshots, so only forward compatibility breaks.
In-repo JS and WASM ship together per release namespace, so the risk is mainly external or archived validators. Either bump the identifier (v3.1 or v4), or document in EPHEMERIS_V3.md that v3 readers must tolerate additive fields.
Generated by Claude Code
| map.sources=map.sources.map((source,index)=>{ | ||
| const original=path.relative(sourceRoot,path.resolve(path.dirname(file.path),source)).split(path.sep).join('/'); | ||
| if(!raw.has(original)||map.sourcesContent[index]!==raw.get(original))throw new Error('Compiler sourcemap source differs from captured input'); | ||
| return 'apps/web/'+original; |
There was a problem hiding this comment.
Low (by reading, not tested in a browser): sources are rewritten to repo-root paths (apps/web/js/x.js), but browsers resolve them relative to the map URL, giving releases/<id>/js/apps/web/js/x.js. That path is not in the manifest, so the service worker answers 409.
DevTools still works because sourcesContent is embedded, but tooling that fetches sources will fail.
Suggested fix: emit ../-relative sources, or set sourceRoot to the archive layout and relax the sourceRoot rejection in validate_release_manifest.py:201 to match.
Generated by Claude Code
| raise ValueError('WASM source materials changed during build') | ||
| document = {'schema_version': 'wasm-build.v1', 'target': 'wasm32-unknown-unknown', | ||
| 'profile': 'release', 'locked': True, 'source_inputs': inputs, | ||
| 'profile': BUILD_PROFILE, 'locked': True, 'source_inputs': inputs, |
There was a problem hiding this comment.
Low: the receipt records the constant BUILD_PROFILE (wasm-release) instead of the profile actually built.
read_receipt also accepts 'release' for receipts whose source_inputs match the current tree (see test_historical_release_receipt_remains_readable). An artifact built with plain --release (opt-level z for the ephemeris instead of 3) therefore passes provenance and silently loses the claimed speedup.
Suggested fix: have build_wasm.py pass the real profile into write_receipt, and accept 'release' only when the Cargo.toml hash matches a known pre-PR one.
Generated by Claude Code
Moon orbital elements now load only when a satellite system is viewed, so the smoke's readiness wait never saw loaded knots on the Sun overview and timed out after 45 s. Request them through the real Focus control (Saturn) before waiting, and fail at once if the moon catalogue reports failure. paint() now queues a frame instead of drawing synchronously. The alias and reset phases read renderer state in the same task as their input, so they saw the previous frame. Each phase now checks only after the first rendered frame that follows its input, using the exported frame timings.
The new reference loader read only a PNG IHDR, so the three JPEG reference assets (Earth land, Earth night lights, Moon) always failed with a source grid mismatch and fell back to the simplified surface. Read the grid from a PNG IHDR or a JPEG frame header, require the container to match the pinned file extension, and decode with the matching media type. Tests check every pinned reference asset against its declared grid, a JPEG decode path, and container/extension/truncation drift.
This change moved the Earth reference-layer note from the stage caption (#destinationCaveat) into the About this view disclosure (#destinationReferenceNote). The caption layout regression still required 'Reference layers:' in the caveat text and failed browser validation. Require the caveat warnings in the caption and the reference note in its new element, and report both texts on failure.
The accuracy line's epoch prefix changes every animated frame. With proportional digits and the slightly larger font in this change, some timestamps fit one fewer wrapped line, so the line's height toggled between frames and browser validation's 1 d/s reflow check failed. Use tabular digits, as the selected-epoch label already does.
Focus selection now inspects a planet at physical relative radii and moon offsets (systemDisplayPolicy.js), but the moon-shadow plan still modeled the enlarged overview: a 0.170 AU Jupiter display radius and systemScale() moon spacing. Its camera was hundreds of times too far out, and it read label anchors in the same task as its repaint, which now only queues a frame, so it saw the previous frame's labels. Use the physical equatorial radius, true moon offsets and physical moon radii, require body inspection explicitly, and read anchors after the queued frame renders. The jupiter-io-transit and control frames pass locally with their existing pixel gates and the half-pixel label residual. The Io eclipse pair still assumes the enlarged moon orbit and is reframed separately.
At physical scale Io orbits 5.9 Jupiter radii out, so the old polar view of Jupiter could keep Io on the canvas only about a pixel wide, below the eclipse gate's 3 px marker floor. Focus Io itself through the real Focus control and view it from the Sun's side: Io's fully lit disc spans half the canvas half-height, stays centred in both frames, and Jupiter lies behind the camera, leaving a black sky. The frame planner rejects any frame in which Jupiter reaches the canvas. With only Io's label on screen, the eclipse assertion now also measures the sunlit disc from pixels and requires its centre within 1 px and its radius within max(1 px, 1%) of the exact perspective silhouette tan(asin(R/d)), which independently checks the camera distance and field of view. The brightness-ratio gate is unchanged. Local Chromium run: eclipse ratio 0.2790 against the shipped 0.2784; lit disc 155.98 px against 155.90 px predicted, centre error 0.21 px; transit umbra 0.0504 against 0.0505 predicted.
The probe focused Earth and then fixed the camera at 0.5 AU, which suited the enlarged Earth. Focus inspection now draws Earth at physical scale, so from 0.5 AU it is about 0.07 px wide, below the 8 px at which the Earth look is demanded; the probe's 30 s readiness wait timed out. Place the whole disc at 30% of the canvas half-height instead, inside assertBlueEarth's central sample. Locally the probe then passes with 2,793 blue pixels against a minimum of 141, and the full browser validation completes.
Auto quality retried the higher tier every 6 s on a device that could not sustain it: a simulated 34 ms-at-High GPU changed tier 35 times in two minutes, and each change resizes the drawing buffer. An upgrade reversed within 15 s now doubles the wait before the next attempt (capped at five minutes); returning to Auto resets it. The same simulation changes tier 9 times, at growing intervals. Upgrades also required continuous animation, so a paused scene lowered during slow dragging never recovered. Fast input-driven frames now count as headroom; idle observations still cannot certify recovery.
Every pointermove over the canvas scheduled a full scene frame before checking for a tracked drag, so hovering kept a paused scene rendering at display rate. Invalidate only for a tracked drag or pinch. Body and moon hit radii dropped to 12 px in this change, which makes small inner planets hard to tap. Touch now keeps a 22 px radius (a ~44 px target); mouse and pen stay at 12 px.
The selected body is laid out first, so crowding cannot remove its label, but near a canvas corner all four adjacent positions cross an edge and its name disappeared. For the selection only, nudge an adjacent box back inside the canvas while it stays within 16 px of its target. Unselected names keep the existing rule and are suppressed rather than detached.
Every frame rewrote each label's display, size and projection data and then read the jump bar's and host's bounds, so a steady frame dirtied layout and forced it again. Write styles and data only when they change, show a label for measurement only when its metrics are missing, and measure the jump-bar overlap once per viewport and visibility state. A repeated frame with unchanged labels now performs no style writes and no layout reads.
Stars fade out between Sun altitudes of -18 and 0 degrees but stayed hover and click targets until fully transparent, and a selected star's path was drawn at full strength in daylight with no visible star. Stars below 25% opacity are no longer hit targets, and a star's path shares its daylight fade. The accessible list still offers every star.
Historical release-profile receipts must stay readable for archived evidence, but validate_receipt also accepted one for the current source tree, which tools/build_wasm.py only ever builds with wasm-release. Reject a non-browser profile there with a rebuild message; read_receipt is unchanged.
After focusing Earth and zooming in, physical-scale inspection fills the whole canvas with Earth. On CI's software GPU the surface and atmosphere passes then cost about 1.1 s per frame (frames every ~2.3 s), so the spin probe saw 2 Earth draws in its unchanged 5 s window and failed on two runs. The spin gates read submitted model and normal uniforms, not pixels, so put the Earth disc at 30% of the canvas half-height first. That shades about 17x fewer pixels and keeps the disc above the 64 px scattering threshold. The 5 s window and the spin contract are unchanged. Locally the probe records 4 draws in 1.5 s and the full browser validation completes.
Outcome
Remediates the structural review findings across rendering, scientific semantics, resource loading and release integrity. Labels stay attached to visible targets, body and satellite-system framing share one policy, and deferred frames/recovery have an explicit lifecycle owner. Catalogue loading, WASM compilation and texture resources are bounded. Exact per-evaluation phase reuse reduces the measured raw-position p95 from 45.3 ms to 8.8 ms under the unchanged 16 ms gate.
This PR targets
precision/verified-readiness-20261109at02aaec8to isolate this remediation from the three preceding baseline commits. It remains a draft because interaction, numerical and coverage qualification described below are still open.Specification and traceability
docs/rfcs/0002-correctness-delivery-and-experience.md.docs/superpowers/plans/2026-11-10-structural-completion.md.docs/structural-remediation.md; scientific dispositions:docs/structural-science-dispositions.md.Correctness and risk
wasm-releaseprofile preserves browser abort behavior while native builds unwind.UI and UX
Validation evidence
Local validation:
build/site-structural-final10, manifest SHA-256183befe0b2fbaa0f6ed87f73d385f43a8abeaa6d66578f0931a3ff360b7451b1. This is local validation over the baseline with working changes, not an artifact built by CI from this new commit.Remaining qualification:
Documentation
Committed-diff review
Fresh independent code review of
02aaec8…f66dc19found no actionable critical/important defects or merge-blocking code regressions in the reviewed scope. It covered scheduling/recovery, camera/picking, catalogue/texture ownership, WASM preparation/cancellation, Sky contracts, Rust phase reuse and bundling/provenance. An independent focused rerun passed 89/89 tests. The remote head and all 182 changed paths match the local commit.At the latest check, Docs, governance, Rust lint, moon-image evidence, cross-OS determinism and the immutable web artifact build passed. Main CI and Coverage workflows are still running; no failed job was reported at that check. See the live PR checks for subsequent results. This review does not close the qualification limits above.