description: "Audit the SpeedTree (.spt) TLV walker + placeholder-billboard fallback shipped in Session 33 Phase 1" argument-hint: "--focus <dimensions> --depth shallow|deep"
SpeedTree Subsystem Audit
Audit the byroredux-spt crate — a young, deliberately small subsystem
(Session 33 Phase 1, "S1"). It does two things: (1) walks the .spt
parameter section as a tag-length-value stream for FNV / FO3 / Oblivion,
and (2) emits a placeholder billboard ImportedScene so TREE cells
render something instead of panicking or going treeless. The real
geometry tail is not decoded — everything past tail_offset is left
on the floor by design.
Keep the audit proportionate. The highest-risk surface is the walker's
byte accounting (one mis-sized payload desyncs the whole stream), then
the placeholder fallback correctness, then TREE record → billboard
wiring, then per-game .spt differences. The tag dictionary is
large but each entry is a fixed-size decode — low risk individually.
Architecture: Single-pass — small enough to run all dimensions inline rather than spawning Tasks.
See .claude/commands/_audit-common.md for project layout, methodology,
deduplication, context rules, severity, and finding format. Do not
re-derive any of those here.
Scope
Crate: crates/spt/src/ — parser.rs, tag.rs, version.rs,
stream.rs, scene.rs, crates/spt/src/import/mod.rs, plus the
feature-gated crates/spt/src/recon/mod.rs. Public surface re-exported
from crates/spt/src/lib.rs: parse_spt, import_spt_scene,
SptImportParams, SptScene, detect_variant, dispatch_tag.
(compute_billboard_size is a private helper in the import module, not
an entry point.)
Cross-cuts (the wiring that actually invokes the crate):
byroredux/src/cell_loader/references/mod.rs— the production route. Anis_sptextension check (model_path … eq_ignore_ascii_case("spt")) dispatches toparse_and_import_spt, which looks up the matching TREE record fromrecord_index.treesand threads its metadata intoSptImportParams.refr.rsdoes not carry a.sptroute.byroredux/src/scene/nif_loader.rs— the--tree/ loose-file direct-visualiser route (parse_import_and_merge,is_sptbranch). This is a parallel path: it callsimport_spt_scenewithSptImportParams::default()— no TREE metadata (no ICON override, no OBND/MODB/BNAM sizing). Verify the two routes don't silently diverge in ways that matter (Dimension 4).crates/plugin/src/esm/records/tree.rs—parse_tree→TreeRecord. Captures OBND / ICON / MODB / SNAM / CNAM / BNAM / PFIG.has_speedtree_binary()is the case-insensitive.sptpredicate. Pre-S1 TREE fell into the generic MODL-only path and dropped every field but MODL.byroredux/src/systems/billboard.rs—make_billboard_systemrotates any entity carrying aBillboardcomponent.BsRotateAboutUp(the mode the spt placeholder uses) currently falls back to the world-up yaw lock — confirm that's still the behaviour and that the-Zfront-face convention inimport/mod.rsmatches the rotation arc used here.byroredux/src/cell_loader/spawn.rs— attaches theBillboardcomponent fromcached.placement_root_billboard, and routes the placeholderImportedMeshthrough the NIFAL boundary (crate::material_translate::translate_material).
Phase 1 ("S1") acceptance (ground truth — verify before reporting):
- TLV walker recovered against the FNV/FO3/Oblivion
.sptcorpus (133 vanilla files; Oblivion ≈ 113). - Acceptance gate ≥ 95 % unknown-tag-clean rate, asserted in
crates/spt/tests/parse_real_spt.rs(env-var gated,#[ignore]). - Placeholder fallback: un-decoded trees render as a billboard card,
never an
Errout of the cell loader. .sptREFRs route to the SpeedTree importer, not NIF.--treesmoke path parses + imports.
Future phases (NOT shipped — do not flag as missing unless --focus
explicitly includes them): real branch/leaf mesh recovery from the
geometry tail, wind-bone animation from SNAM/CNAM, distance-LOD swap,
baked-shadow lookup. SNAM/CNAM are parsed but not consumed (TD5-011
gate) — that's intentional, not a drop.
Parameters (from $ARGUMENTS)
--focus <dimensions>: comma-separated dimension numbers. Default: all.--depth shallow|deep:shallow= walker contract + wiring review from source only;deep= also run the corpus harness against on-disk BSAs. Default:deep.
Extra Per-Finding Fields
- Dimension: Walker Byte-Accounting | Placeholder Fallback | TREE→Billboard Wiring | Per-Game Variants | Tag Dictionary | NIFAL Material Translation
Phase 1: Setup
mkdir -p /tmp/audit/speedtree- Dedup query (per
_audit-common.md):gh issue list --repo matiaszanolli/ByroRedux --limit 200 --json number,title,state,labels --search "speedtree OR spt OR TREE" > /tmp/audit/speedtree/issues.json - Read the most recent
docs/audits/AUDIT_SPEEDTREE_*.mdreport (sort by date — do not hardcode a filename here, it rots every cycle) and diff direction against it rather than re-deriving from scratch. Findings SPT-D4-01/02/03/04, SPT-D5-01/02, SPT-D2-01, SPT-D3-01, SPT-D1-01 are all closed (#994/#995/#996/#997/#998/#999/#1000/#1001/#1002). Of the later SPT-NEW batch, SPT-NEW-01 (dead-codedetect_variant, #1820) and SPT-NEW-06 (format-notes byte-align, #1821) are also closed — only SPT-NEW-07 (MaybeStringElseBaremisparse risk on a bare tag-13005 immediately before the geometry tail, #1822) remains open. Treat closed findings as regression guards, not open items; only re-flag if a guard has actually broken. deeponly — corpus + recon harness:- corpus location:
Fallout - Meshes.bsa(FNV/FO3),Oblivion - Meshes.bsa. - acceptance run:
BYROREDUX_FNV_DATA=… cargo test -p byroredux-spt --release --test parse_real_spt -- --ignored --nocapture(and_FO3_DATA/_OBL_DATA). - per-file dumps: the
reconexamples are feature-gated —cargo run -p byroredux-spt --features recon --example spt_walk(alsospt_tagmap,spt_transitions,spt_dissect,spt_recon). Findings log tocrates/spt/docs/format-notes.md.
- corpus location:
Phase 2: Dimensions
Ordered by SpeedTree risk.
Dimension 1: Walker Byte-Accounting
Highest risk. The walker has no length-prefixed framing for most tags —
it advances by the payload kind's fixed size. A single wrong size in the
dictionary desyncs every subsequent read.
Entry points: crates/spt/src/parser.rs (parse_spt, read_payload,
TAG_MIN/TAG_MAX), crates/spt/src/stream.rs (SptStream LE readers +
is_eof/remaining guards), crates/spt/src/parser.rs tests.
Checklist:
- Each
SptTagKinddecode advances the cursor by exactly the bytes it claims:U8=1,U32=4,Vec3=12,FixedBytes(n)=n,String=4+len,ArrayBytes{stride}=4+count×stride,Bare=0. Cross-checkread_payloadagainst the dictionary intag.rsfor any size mismatch. - The
MaybeStringElseBarebranch (tag 13005, #999) consumes the tag, then peeks the next u32 to decide Bare vs String. Confirm it re-syncs cleanly on both arms and that aNonepeek (EOF) can't panic — thetag_13005_at_eof_does_not_panicregression guard. - Walker stops cleanly at
is_eof()(setsreached_eof) and at an out-of-range /Unknowntag (recordstail_offset+unknown_tags), never reads one tag past EOF. - Pathological lengths:
read_string_lpandArrayBytesboth cap at 64 KiB and bail withErr, not OOM. Confirm the cap is on the byte count (count × stride), not justcount. parse_sptreturnsErronly on the two fatal conditions (missing magic, mid-payload underflow). In-range-but-unknown tags are non-fatal (recorded, walker stops) — this is the contract the placeholder relies on. Any new fatal-error path is a HIGH finding (it kills the cell-loader fallback).- Endian: LE-only, unconditional (no version-gated readers — every
.sptis__IdvSpt_02_). Flag any big-endian assumption or host-endian read.
Dimension 2: Placeholder Fallback Correctness
Entry points: crates/spt/src/import/mod.rs (import_spt_scene,
compute_billboard_size, placeholder_billboard_mesh,
placeholder_root_node, DEFAULT_BILLBOARD_WIDTH/_HEIGHT),
byroredux/src/systems/billboard.rs.
Checklist:
import_spt_scenealways returns a one-node / one-meshImportedScene— there is noErrpath out of it. The only way the cell loader getsNoneisparse_sptreturningErr(magic / underflow); confirm that path logs + skips the REFR without aborting the rest of the cell (graceful-degradation contract).- Leaf-texture precedence: TREE.ICON override →
.spttag 4003 (first wins, #997) → unset (renderer magenta placeholder). Regression guards:leaf_texture_override_wins_over_spt_tag,empty_texture_leaves_path_unset_for_renderer_placeholder. - Billboard sizing precedence in
compute_billboard_size: OBND → BNAM → MODB → 256×512 default, every path clamped to[16, 8192](#1001/#1002). OBND beats BNAM intentionally (BNAM clamps tall trees, e.g. WhiteOak01 OBND 802×1567 vs BNAM 768×768). Vanilla Oblivion ships MODB-only / no OBND, so an OBND-first-or-default path would render Cyrodiil pines at half scale. Guard the ordering. - Normal / winding convention (#1000): front-face normal is
-Z, indices[0, 3, 2, 2, 1, 0]. The billboard system rotates viaQuat::from_rotation_arc(-Z, look_dir), so object-Zends up facing the camera — the textured face must be-Z. Pre-#1000 normals were+Zandtwo_sided: truemasked it. Guards:placeholder_normals_point_negative_z_for_billboard_arc,placeholder_index_winding_produces_negative_z_geometric_normal. bs_boundZ-up → Y-up swap (#995): center viabyroredux_core::math::coord::zup_to_yup_pos, half-extents reshuffled(hx, hz, hy). Guard:placeholder_uses_obnd_bounds_when_present.- Two-sided + alpha-test cutout:
two_sided: true,alpha_test: true,alpha_threshold: 0.5,alpha_test_func: 6(GREATEREQUAL),has_alpha: false(cutout and blend are exclusive). BsRotateAboutUphandling inbillboard.rs::compute_billboard_rotationcurrently falls back to the world-up yaw lock (it lacks the local frame). Verify that approximation is still acceptable for tree imposters and documented — drifting it silently into pitch would tilt every tree.
Dimension 3: TREE → Billboard Wiring
Entry points: byroredux/src/cell_loader/references/mod.rs
(is_spt dispatch, parse_and_import_spt),
byroredux/src/cell_loader/spawn.rs (placement_root_billboard →
Billboard::new), crates/plugin/src/esm/records/tree.rs (parse_tree,
TreeRecord, has_speedtree_binary).
Checklist:
- The
.sptroute fires when the REFR's TREE base's MODL ends in.spt; TREE record is fetched fromrecord_index.treesby the same form id resolved againstindex.statics. Mixed.nif+.sptREFRs in one cell must coexist. parse_and_import_sptreturns the sameCachedNifImportshape as every other model, with synthetic defaults the generic spawn path must not mis-read as NIF-rooted:placement_root_billboard = Some(BsRotateAboutUp)(#994),bsx_flags = 0(#1214),root_flags = 0(#1235),flame_attach_offset = None. Confirm the spawn site never assumes the placeholder carries a real NiAVObject root / BSXFlags / flame marker.spawn.rsactually inserts theBillboardcomponent whenplacement_root_billboard.is_some()— without it the quad spawns static (this was SPT-D4-01, now closed; it's the regression guard for the whole dimension).TreeRecordfield capture is lossless for the fields the importer reads (OBND→bounds, ICON→leaf_texture, MODB→bound_radius, BNAM→billboard_size). SNAM/CNAM are parsed-but-not-consumed (TD5-011) — don't flag as a drop, but DO flag if they're silently mis-parsed (e.g. CNAM length not shape-tolerant across the 5-float Oblivion vs 8-float FO3/FNV split)..sptfiles in BSAs resolve through the sameextract_meshlookup chain as.nif(sibling-BSA auto-load, AE pipeline-path strip if relevant) — no parallel "spt resolver".- Cell unload despawns the placeholder entities cleanly; no leaked BLAS entries for the billboard quad.
Dimension 4: Per-Game Variants & Route Divergence
Entry points: crates/spt/src/version.rs (detect_variant,
SpeedTreeVariant, MAGIC_HEAD), byroredux/src/scene/nif_loader.rs
(parse_import_and_merge is_spt branch).
Checklist:
detect_variantrecognises any__IdvSpt_02_-prefixed file but cannot tell V4Oblivion from V5Fnv at the magic level — it defaults toV5Fnvand the caller is meant to override via game context. Verify nothing downstream actually depends on the variant being correct today (the placeholder path is variant-agnostic); if a consumer branches on it, that's a real bug. Guards:detect_variant_recognises_idvspt_magic,detect_variant_unknown_for_non_speedtree_inputs.MAGIC_HEADis the exact 20 bytes (u32 1000,u32 12,"__IdvSpt_02_"). A one-byte flip must reject → placeholder. Confirm no partial-prefix leakage (input shorter than 20 bytes rejects).- Route divergence (the real per-game risk here): the cell-loader
route threads TREE metadata; the
--tree/ loose route usesSptImportParams::default(). That means a loose-loaded Oblivion.sptgets the 256×512 default (no MODB sizing) and no ICON override. Confirm this is understood/documented and isn't masking a sizing bug that would also bite the cell route. Flag if the two routes have drifted in the parse call (they must both callparse_spt+import_spt_scene). - Oblivion (SpeedTree 4.x) vs FO3/FNV (5.x): the parameter walker is
assumed unified across all three (same magic, same tag dictionary). The
geometry-tail layout is not confirmed unified — but the tail is
out-of-scope for Phase 1, so only flag a tail-decode assumption if
--focusincludes the future phases.
Dimension 5: Tag Dictionary
Lower risk (fixed-size decodes), but a wrong size here is the Dimension-1
desync trigger, so spot-check rather than skip.
Entry points: crates/spt/src/tag.rs (SptTagKind, dispatch_tag),
crates/spt/docs/format-notes.md, recon examples (spt_tagmap,
spt_transitions).
Checklist:
dispatch_tagcurrently maps ~90 tag values across the payload kinds. This is conservative-by-design: any tag not in the table →Unknown→ walker stops cleanly. The old "~14 tags / 40-tag aspirational target" framing is stale — do not report dictionary size as a gap.- Cross-check a sample of fixed-size assignments against the
format-notes.md2026-05-09 table and thetag.rsunit tests (fixed_byte_payload_tags,string_payload_tags, etc.): e.g. 8003/8005/8009 = 52 B, 13008 = 11 B, 13013 = 7 B, 12002 = 16 B, 12003 = 20 B, ArrayBytes 10002 stride 1 / 10003 stride 8. A size that contradicts the observed histogram is a Dimension-1 desync waiting to happen → MEDIUM. - Confounder tags (
4096,5376, string-length values that fell in the tag band) must stayUnknownso the walker bails rather than misparses (unknown_for_out_of_dictionary_tags). - Any tag observed in the corpus at ≥1 % frequency that's still
Unknownshould have aformat-notes.mdrationale; an undocumented common-tag bail is a LOW finding.
Dimension 6: NIFAL Material Translation for Placeholders
The placeholder ImportedMesh flows through the single NIFAL boundary like
any other mesh. Cross-cuts /audit-nifal — report single-boundary /
no-fabrication findings there, not here.
Entry points: the material defaults in
crates/spt/src/import/mod.rs (placeholder_billboard_mesh),
byroredux/src/material_translate.rs (translate_material, consumed at
the spawn.rs call site), crates/core/src/ecs/components/material.rs
(Material::resolve_pbr).
Checklist:
- The placeholder is canonicalised at the single
translate_materialboundary — no parallel "spt material" path that bypasses it (the--treeloose route and the cell route must both land here viaspawn.rs). - Non-PBR defaults survive translation:
is_pbr: false,from_bgsm: false(#1076/#1077);metalness_override: Some(0.0)/roughness_override: Some(0.85)— explicit foliage defaults set at import (#1819/SPT-NEW-05,placeholder_billboard_meshincrates/spt/src/import/mod.rs), NOTNone. A regression toNonere-opens the keyword-classifier substring collision (Boxwood→wood, Elderberry→glass). Guard:placeholder_billboard_sets_foliage_pbr_overrides_regardless_of_texture_path.resolve_pbrmust fill the canonicalmetalness/roughnessf32 from the non-PBR keyword path, never promote the billboard to metallic-roughness. SpeedTree never resolves a BGSM/BGEM (#1241/#1353) — guard that import-side PBR plumbing (a82366e9-style) left the billboard non-PBR. emissive_source: EmissiveSource::None(#1280) holds — a tree billboard must not pick up an emissive lobe.- The two-sided alpha-test cutout maps to the correct canonical
Materialflags after translation (foliage silhouette preserved, not opaque-blitted).
Phase 3: Output
Write findings to docs/audits/AUDIT_SPEEDTREE_<TODAY>.md using the
base finding format from _audit-common.md. Mark anything already covered
by #994–#1002 as a regression guard, not a new finding. Suggest
/audit-publish on completion.
TDD Red-Green-Refactor
Testing
Skill that guides Claude through the complete TDD cycle.
Web Accessibility Audit
Testing
Performs a comprehensive web accessibility audit following WCAG standards.
UAT Test Case Generator
Testing
Generates structured and comprehensive user acceptance test cases.