vibe-view code audit — 2026-07-02

Full-codebase audit of vibe-view (most of which was originally written by a weaker model), covering: core QVF/converters/API, renderers + scientific numerics, exporters, the CLI, the trame UI wiring, both UIs live (web via trusted-event Playwright, desktop via the Electron app), output correctness, and dependency currency.

Method

  • Four parallel read-only audit passes (core, renderers/numerics, exporters, CLI) with empirical verification of every finding.

  • Template/UI-wiring scan: the live Vue template dumped and every expression syntax-checked; trigger('...') calls cross-checked against the trame trigger registry; v-model targets cross-checked against state keys.

  • Live UI: docs/interact_controls.py (trusted-event Playwright) — 5/5 interactive checks pass (measure picking, trajectory, reaction path, vibrations, clip plane). Desktop app driven natively — export buttons exercised and their outputs validated (XYZ/CML/POV/GLTF correct).

  • Dependencies: Python stack current (pyvista 0.48.4, trame 3.13.2, vtk 9.6.2, pydantic 2.13.4); Electron v43.0.0 = latest stable; four stale transitive packages updated (pillow, narwhals, aiohappyeyeballs; pydantic-core stays at the version pydantic pins).

Fixed in this audit (commit series of 2026-07-02)

~35 confirmed defects fixed, each with empirical verification and, where cheap, a regression test. Highlights by severity:

Science-wrong output (critical):

  • Generated vibe-qc input scripts embedded Å as bohr (geometries ran 0.529× compressed); the periodic template had a NameError (BasisSet not imported) and emitted lattice rows where PeriodicSystem takes columns.

  • vibe-view open input.py stored parsed bohr as Å (1.89× inflated, no bonds); keyword forms (Molecule(atoms=…), PeriodicSystem(lattice=…)) parsed zero atoms; cell = np.array(...) variable lattices resolved to identity.

  • Trajectory/vibration “videos” were static (every frame re-read the stored geometry); orbital animation crashed on import, double-converted units, and could never render the negative lobe.

  • replicate_cell collapsed all replicas onto the base image and stretched by the supercell; supercell/CIF-export/miller_slab used a transposed lattice convention; h-add dropped the lattice.

  • Periodic covalent-bond inference was dead code (cross-boundary bonds missing); bonds did not replicate with the cell.

  • Blender export generated non-runnable Python (null/false literals); SVG/PDF export collapsed planar molecules to a line (now PCA projection); CIF export wrote Cartesian Å into _atom_site_fract_* with no cell.

Broken features (major):

  • All keyboard shortcuts (e/m/r/s/p, Ctrl+Z/Y, Delete) were dead — FOUR stacked causes, each individually fatal and only discoverable live: (1) the handlers were registered with @ctrl.set only, which creates no client trigger; (2) the keydown/contextmenu/tooltip installer JS was injected via ClientStateChange handlers whose trigger states never change before the listener exists, so the listeners were never installed (now client.Script tags with canvas-mount retry); (3) the raw JS mutated state via property assignment (t.state.x = v), which is a silent no-op in trame’s client API — only t.state.set() reaches the reactive state (broke ?, v, context menu, tooltips); (4) the canvas selector .vtk-view canvas matches nothing in this trame-vtk version, and vtk.js overlays an interaction layer so contextmenu events never reach canvas-level listeners anyway (context menu now binds on document with a canvas-bounds check). All four paths live-verified with trusted Playwright input: ? opens the help dialog, m toggles measure mode, right-click opens the context menu, hover updates the tooltip state.

  • diff’s verdict sentence was sign-inverted; batch-compare energies were always empty; align=True didn’t superpose (index-wise RMSD instead of Kabsch).

  • Material presets repainted the whole scene uniform grey; space-filling used covalent radii while claiming vdW.

  • A config file silently overrode explicit --port/--host.

  • Electronic-DOS lazy reads always failed (dispatched to the phonon reader).

  • slice couldn’t select dotless kinds and silently ignored --keep when --drop was present; capture -s <structure> was refused; MO cubes were read with data shifted by the DSET line; SDF counts parsing crashed or dropped all bonds; in-memory slice_qvf produced manifest-only archives.

Known issues — deferred (need design decisions or larger rework)

These are real, verified problems that were not fixed in this pass because they need either a maintainer decision or a non-trivial rewrite.

  1. Point-group detection is orientation-dependent (symmetry.py). Water rotated arbitrarily → C1 (aligned → C2v); CH₃CN → Cs (should be C3v); staggered ethane → C2h (should be D3d); X₆ octahedron → Th (count heuristic). Correct fix: transform to the principal-axis frame, classify the rotor type on all three moments, search candidate C2 axes/mirror planes from atom geometry (not the Cartesian axes), and discriminate Dnd/Dnh via σh. This is a rewrite of ~200 lines of algorithmic code. Until then, treat the symmetry panel’s point group as approximate for non-axis-aligned input.

  2. ~~miller_slab is only correct for (001) on c-orthogonal cells (crystal_builder.py): the “plane distance” is not the true d-spacing (needs the reciprocal metric) and vacuum is always added along c.~~ Resolved 2026-07-28. The spacing now comes from the reciprocal lattice (d = 1/|inv(cell) @ hkl|); the old 1/|(h,k,l)| was dimensionless and gave 1.0 A for a cubic 5 A cell’s (001). Slab thickness and layer selection both inherited it and are fixed with it. The vacuum-along-c limitation is now enforced rather than silent: an orientation whose normal is not along c raises, because keeping a and b only yields a slab when they lie in the (hkl) plane, and a general surface cell needs two lattice vectors spanning it. Tests: tests/test_miller_slab.py.

  3. Path-traced rendering misuses the OSPRay API (raytrace.py): AddMaterial/AddTexture receive tuples where VTK expects material implementation names / vtkTexture; denoiser + depth-of-field calls don’t match the renderer-node API; the “three-point lighting” creates three camera headlights. Only testable against an OSPRay-enabled VTK build, which the shipped wheel is not.

  4. ~~setup_shadow_pass never adds the shadow-map pass to the chain (material_presets.py) — shadows silently don’t render.~~ Resolved by removal (2026-07-17): the helper was also never called from anywhere, and being a renderer.SetPass effect it could not have reached the client’s vtk.js viewport even once fixed (only server-side output — screenshots / HQ render — sees renderer passes). Deleted; the client-vs-server rendering boundary is now documented in material_presets.py’s module docstring.

  5. ~~3D atom labels are not camera-facing (renderers/structure.py build_label_mesh): fixed-orientation Text3D meshes appear mirrored / oversized from most camera angles (verified in the desktop app). The design constraint is real — vtk.js’s VtkLocalView doesn’t render VTK 2D label actors, which is why 3D text was chosen. A fix needs a vtk.js-compatible billboard strategy (e.g. re-orienting the label mesh on camera change server-side, or per-label follower emulation).~~ Resolved 2026-07-28 (workstream D5, HANDOVER.md): a client-to-server camera readback (ctrl.sync_client_camera) lets build_label_mesh(camera=...) re-orient labels on interaction end. Covered by tests/test_billboard_labels.py and tests/test_camera_sync.py. See ROADMAP_AVOGADRO_PARITY.md § 6 D5.

  6. ~~share.py is an orphangenerate_share_url mints https://vibe-qc.com/share/<token> URLs no server route serves, and the module is imported nowhere. Ship a /share/ route or delete the module.~~ Resolved (2026-07-25, maintainer decision): deleted. Shipping the route as written would have meant hosting users’ unpublished research geometries on a public server — a service with a real privacy surface, not a viewer feature. The module, its tests, and the tutorial section documenting an API that never worked are gone.

  7. ~~file_watcher.py compares only mtime+size and can fire mid-write on partially written QVFs (no content hash / settle delay).~~ Fixed (M2, 2026-07-02): rewritten around a content fingerprint (per-section manifest digests) with a settle delay, a mid-write hold on unparseable archives, and a per-section diff consumed by the viewer’s hot-reload (tests/test_file_watcher.py). The same milestone fixed the auto-reload path reusing the stale open zip handle instead of reopening the file.

  8. Converter approximations: cif_to_qvf ignores _symmetry_equiv_pos (asymmetric-unit-only CIFs convert incomplete, no warning); pdb_to_qvf treats the placeholder CRYST1 1 1 1 record as a real 1 Å cell; xyz_to_qvf flattens multi-frame XYZ into superimposed atoms.

  9. desktop.py’s generate_electron_* scaffolding is stale — it generates a v1.8-era main.js/package.json that duplicates (and now contradicts) the maintained electron/ app. Candidates for deletion.

  10. config.py [display] keys (size, representation) are read nowhere; compare doesn’t consult the config although the config comment claims it applies.

  11. lazy_loader.get_memory_usage reports sys.getsizeof of wrapper objects (~48-100 B) for multi-MB sections — the UI memory panel numbers are meaningless.

  12. api.get_volume returns only grid metadata + min/max, not the data array its docstring promises (API-shape decision).

  13. wavefunction.py skips l>3 shells during MO evaluation (documented in-module; surfaced as last_dropped_l_fraction).

UI verification results

  • Web (Playwright, trusted events, all_sections.qvf): measure-mode picking, trajectory play, reaction-path play, vibration play, clip enable — 5/5 pass.

  • Desktop (Electron, live): browse → open → 3D render, folder navigation, File menus, Open Recent, export buttons GLTF/XYZ/CIF/POV/BLDR/SVG/CML all fire and write next to the QVF (plus a data-URL download); XYZ, CML, POV, GLTF outputs validated against the structure. Exports land as <stem>_export_<timestamp>.<ext> next to the source file.

  • Keyboard shortcuts work after the trigger-registration fix.