feat(cesium): implement tileset.rs cold-import parser — Group-A entry, no-serde#205
Conversation
…p-A entry) Fill the commented tileset.rs scaffold with a live OGC 3D Tiles 1.1 parser per the crate's RULE 3 (no serde, no json crates): a dependency- free hand-rolled tokeniser parses at the cold boundary into a transient JSON model that is walked into owned structs and dropped before return — no JSON value reaches the hot path. - Owned model: Tileset / TilesetAsset / TileNode / enum BoundingVolume / Refine / TileContent / ImplicitTilingRef / SubdivisionScheme / ParseError. - content+contents unified into one Vec; refine inheritance resolved at traversal; uri/legacy-url accepted; box/region/sphere dispatch. - 10 unit tests; 67/67 crate tests pass; tileset.rs clippy-clean. Fills the entry point of the Group-A ingest front-end (khr_gs.rs and implicit_tiling.rs remain scaffold). Refactored from the audit session's reader logic onto cesium's owned-struct contract. Proposed for Opus + CodeRabbit review per the crate's scaffold contract. https://claude.ai/code/session_01UHfLwSzGfVNzkNrTEBijuA
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds a complete, dependency-free ChangesTileset JSON Parser Implementation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b5c0edaba2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| pub fn parse_tileset(bytes: &[u8]) -> Result<Tileset, ParseError> { | ||
| let text = std::str::from_utf8(bytes).map_err(|_| ParseError::InvalidUtf8)?; | ||
| let mut parser = JsonParser::new(text.as_bytes()); | ||
| let doc = parser.parse_value()?; |
There was a problem hiding this comment.
parse_tileset parses one JSON value and then immediately maps fields from it, but never checks that the parser reached end-of-input. This accepts malformed payloads where a valid tileset.json object is followed by extra bytes, so concatenated/corrupted inputs bypass JsonSyntax rejection and are treated as valid tilesets.
Useful? React with 👍 / 👎.
| if let Some(b) = v.get("box") { | ||
| Ok(BoundingVolume::Box(parse_f64_array::<12>(b, "boundingVolume.box")?)) | ||
| } else if let Some(r) = v.get("region") { | ||
| Ok(BoundingVolume::Region(parse_f64_array::<6>(r, "boundingVolume.region")?)) |
There was a problem hiding this comment.
parse_bounding_volume picks the first present key (box, then region, then sphere) and succeeds even if multiple shape keys are present in the same object. Because 3D Tiles requires exactly one shape, malformed inputs can silently select the wrong bounds and change culling/selection behavior instead of failing fast.
Useful? React with 👍 / 👎.
| let subtree_levels = v | ||
| .get("subtreeLevels") | ||
| .and_then(Json::as_f64) | ||
| .ok_or(ParseError::MissingField("implicitTiling.subtreeLevels"))? as u32; | ||
| let available_levels = v |
There was a problem hiding this comment.
subtreeLevels and availableLevels are read as f64 and cast with as u32 without range/integer checks. Inputs like negative numbers, fractions, or NaN are silently coerced (e.g., to 0 or truncated), violating the documented >= 1 integer contract and allowing invalid implicit-tiling metadata to propagate instead of returning a parse error.
Useful? React with 👍 / 👎.
2 participants
Summary
Fills
crates/cesium/src/tileset.rs— the one Group-A ingest module still commented scaffold — with a live OGC 3D Tiles 1.1tileset.jsonparser, per the crate's RULE 3 (Cargo.toml: no serde, no json crates).Jsonmodel that is walked into owned structs and dropped before return — no JSON value survives the import boundary.Tileset/TilesetAsset/TileNode/ enumBoundingVolume(Box·Region·Sphere) /Refine/TileContent/ImplicitTilingRef/SubdivisionScheme/ParseError.content+contentsunified into oneVec;refineinheritance resolved at traversal;uri(1.1) and legacyurl(1.0) both accepted; fail-fast on missing fields / conflicting content / unknown enums.iter_leaves,collect_content_uris, refine-resolvedvisit_preorder,node_count.Refactored from the audit session's tested serde reader onto cesium's owned-struct, no-serde contract — the serde layer replaced by the hand tokeniser; the standalone
ada-3dtilesprototype is dropped (the reader belongs here, in thecesium/3dgsline).Scope / boundary
Cargo.tomlchange — stays dependency-free..3tzZIP ingestion deferred (needs a ZIP reader + OGC 22-025r4 §13 grounding); theArchive*ParseErrorvariants are reserved for it.khr_gs.rsandimplicit_tiling.rsremain scaffold (separate follow-up;khr_gs'sSH_DEGREE_nattrs are thesh.rssurface the ASG-leaf thread rides).Test plan
cargo test -p cesium→ 67/67 pass.tileset.rsis clippy-clean (0 errors attributable to this file).Heads-up — pre-existing clippy debt (NOT from this PR)
cargo clippy -p cesium --all-targets -- -D warningscurrently fails on 9 errors that predate this change (merged via #202/#204), surfaced only under--all-targets:fixtures.rs:412–415— 4×assertions_on_constants(wrap inconst { assert!(..) }).to_cam_soa.rs:518–519—erasing_op("this operation will always return zero") — possibly a real logic slip, worth a look — not a mechanical#[allow].Per the crate's scaffold contract, this impl is submitted for Opus + CodeRabbit review before being considered "uncommented."
https://claude.ai/code/session_01UHfLwSzGfVNzkNrTEBijuA
Generated by Claude Code
Summary by CodeRabbit