text component codec#81
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughWorkspace manifests, helper crates, NBT types, protocol schemas, and text schemas are updated to use new raw, ref, and tag-backed representations, with build scripts and derive macros refactored around new lookup and token-generation paths. ChangesPublication metadata and release settings
Shared utility APIs
Generators and derive macros
NBT model and parsing
Protocol raw types and packet schemas
Text primitives and events
Text dialog and profile schemas
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
haya_protocol/src/item_stack.rs (1)
122-123: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winKeep the optional item stack empty sentinel symmetric.
Line 122 accepts
count > 0withid == item::air, but the writer canonicalizesairto count0, dropping the patch payload on round-trip. Lines 164 and 189 also only checkcount == 0, while the reader treats everycount <= 0as empty; a negative public count with a non-air id writes trailing item data that the reader will not consume.🐛 Proposed fix
let id = item::read(buf)?; + if id == item::air { + return Err(Error); + } let positive = V32::read(buf)?.0 as usize; let negative = V32::read(buf)?.0 as usize; @@ - if count == 0 || id == item::air { + if count <= 0 || id == item::air { V32(0).write(w); @@ - if count == 0 || id == item::air { + if count <= 0 || id == item::air { V32(0).len_s()Also applies to: 164-190
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@haya_protocol/src/item_stack.rs` around lines 122 - 123, The optional item stack sentinel handling in ItemStack::read and ItemStack::write is inconsistent: the reader accepts air with a positive count and treats any count <= 0 as empty, while the writer only canonicalizes count == 0 to air, which can drop payloads or leave unread trailing data. Make the empty-stack representation symmetric by normalizing all non-positive counts to the empty sentinel on write, and ensure the read/write logic in ItemStack::read, ItemStack::write, and the related count checks around the optional stack paths treat air and empty counts consistently so round-tripping preserves or correctly discards payloads.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@haya_data/build.rs`:
- Around line 70-86: The range calculations in build.rs are using the raw byte
count returned by read() instead of the growing data buffer length, so slices
can drift when read() inserts separator newlines. Update the range construction
for reg, flu, blo, ite, ent, and pac to derive each start/end from data.len()
before and after each read call, using the existing read() helper and the same
range variables to keep each generated slice aligned.
In `@haya_data/Cargo.toml`:
- Line 13: The Cargo manifest publish setting uses the wrong registry alias;
update the `publish` entry in `Cargo.toml` to use the canonical crates.io
identifier. In the manifest’s `publish` field, replace the current registry name
with `crates-io` so Cargo recognizes the default registry correctly.
In `@haya_ecs/Cargo.toml`:
- Line 9: The crate is inheriting `publish` from the workspace, but
`workspace.package.publish = false` makes `haya_ecs` non-publishable. Update the
`publish` setting in `Cargo.toml` for this crate to explicitly allow publishing,
or adjust the workspace default and override any internal crates as needed. Use
the `publish.workspace` entry in this manifest and the workspace package publish
configuration to locate the change.
In `@haya_ident/src/lib.rs`:
- Around line 207-222: Identifier::new is incorrectly returning None for
byte-valid paths that exceed the inline HayaStr capacity. Update the constructor
to mirror Ident::to_identifier by falling back to a Heap-backed Inner variant
when HayaStr::copy_from fails, while still rejecting invalid bytes via
is_valid_path. If const-ness must be preserved, keep the current behavior only
if all Identifier::new callers are guaranteed to stay within 31 bytes and
document that constraint.
In `@haya_nbt/Cargo.toml`:
- Line 25: The rand dependency is too loosely specified, which can pull in
unexpected 0.x updates and break reproducibility. In Cargo.toml, update the rand
entry to a pinned minor version based on the currently resolved version in your
environment, and keep it aligned with any code that uses rand APIs so the crate
resolves consistently across builds.
In `@haya_nbt/src/stringify.rs`:
- Around line 557-559: The escape-sequence write in the stringification path is
overwriting the start of the buffer instead of appending to its spare capacity.
In the char-encoding logic inside the stringify routine, adjust the
`char::encode_utf8` call to target the unused tail of `buf` rather than a slice
starting at `buf.as_mut_ptr()`, then extend the length based on the number of
bytes written. Use the existing `buf` handling in `stringify.rs` to locate and
update this append path.
In `@haya_protocol/Cargo.toml`:
- Around line 3-10: The crate-level publish setting is inherited from the
workspace default, so `haya_protocol` is currently configured as
non-publishable. Update the `publish` entry in the crate manifest to explicitly
reflect the intended release behavior for `haya_protocol` rather than relying on
`publish.workspace = true`, and ensure the manifest clearly matches whether this
package should be published or kept internal.
In `@haya_ser_macro/Cargo.toml`:
- Line 11: The Cargo manifest for haya_ser_macro uses an undefined registry name
in its publish setting, so update the package metadata to allow publishing to
the default registry instead of referencing crates.io. Locate the publish field
in Cargo.toml and change it from a registry list to the boolean form, keeping
the rest of the package metadata unchanged.
In `@haya_ser_macro/src/lib.rs`:
- Around line 95-99: Matching on i.data in the serialize path partially moves
the input and makes i unusable afterward; update the logic in the serializer
entry point to match on &i.data so the full ItemStruct/ItemEnum value can still
be passed into serialize::serialize_struct and serialize::serialize_enum. Apply
the same borrowing fix in the deserialize path as well, using the corresponding
deserialize dispatch function(s) in lib.rs to avoid the same compile error
there.
In `@haya_str/src/lib.rs`:
- Around line 148-150: The `as_mut_str` method in `haya_str::Str` is exposing
the full backing buffer instead of the logical string contents. Update it to
build the mutable `str` from only the initialized prefix tracked by `self.len`,
matching the behavior of `len()` and `copy_from`. Use the existing `as_mut_str`
symbol and the `self.len` field to locate the change, and ensure the returned
slice never includes the zero-padding tail.
In `@haya_text/src/chat.rs`:
- Around line 846-866: The decoration parsing in chat.rs is dropping values
because DecorationMap builder methods return a new instance instead of mutating
in place. Update the BOLD, ITALIC, UNDERLINED, STRIKETHROUGH, and OBFUSCATED
branches to assign the result of each with_* call back into style.decorations so
the parsed flags are preserved.
In `@haya_text/src/color.rs`:
- Around line 35-43: The FromStr implementation in color.rs is too permissive
because it parses any hex length after stripping the leading #, so it accepts
inputs like `#abc` and `#0000000`. Update the parsing logic in the FromStr path for
Color to require exactly six hex digits before calling u32::from_str_radix,
matching the Deserialize behavior for `#RRGGBB` and keeping the existing Error
handling unchanged.
In `@haya_text/src/decoration.rs`:
- Around line 29-38: The decoration setters are returning updated values, but
the deserialization code is discarding them, so decoded bold/italic/etc. never
persist; update the call sites that use the with_* methods to assign the
returned DecorationMap back, using the relevant helpers like with_obfuscated and
the other with_* setters. Also fix the None case in with_obfuscated so it does
not set both bits and make is_empty() false; preserve the “unset” state
consistently with the getter behavior.
---
Outside diff comments:
In `@haya_protocol/src/item_stack.rs`:
- Around line 122-123: The optional item stack sentinel handling in
ItemStack::read and ItemStack::write is inconsistent: the reader accepts air
with a positive count and treats any count <= 0 as empty, while the writer only
canonicalizes count == 0 to air, which can drop payloads or leave unread
trailing data. Make the empty-stack representation symmetric by normalizing all
non-positive counts to the empty sentinel on write, and ensure the read/write
logic in ItemStack::read, ItemStack::write, and the related count checks around
the optional stack paths treat air and empty counts consistently so
round-tripping preserves or correctly discards payloads.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5bd4174f-a6b5-484b-b602-c6ae753a6db5
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (71)
.cargo/config.tomlCargo.tomlhaya_collection/Cargo.tomlhaya_collection/src/lib.rshaya_data/Cargo.tomlhaya_data/build.rshaya_data/src/lib.rshaya_ecs/Cargo.tomlhaya_ecs/src/lib.rshaya_ident/Cargo.tomlhaya_ident/src/lib.rshaya_math/Cargo.tomlhaya_math/src/lib.rshaya_mutf8/Cargo.tomlhaya_mutf8/src/lib.rshaya_nbt/Cargo.tomlhaya_nbt/build.rshaya_nbt/src/compound.rshaya_nbt/src/lib.rshaya_nbt/src/list.rshaya_nbt/src/number.rshaya_nbt/src/string.rshaya_nbt/src/stringify.rshaya_nbt/src/unicode.rshaya_palette/Cargo.tomlhaya_palette/src/lib.rshaya_protocol/Cargo.tomlhaya_protocol/src/advancement.rshaya_protocol/src/chat.rshaya_protocol/src/clientbound.rshaya_protocol/src/clientbound/common.rshaya_protocol/src/clientbound/game.rshaya_protocol/src/clientbound/login.rshaya_protocol/src/crafting.rshaya_protocol/src/entity.rshaya_protocol/src/entity_data.rshaya_protocol/src/item_stack.rshaya_protocol/src/item_stack/item_attribute_modifiers.rshaya_protocol/src/lib.rshaya_protocol/src/map.rshaya_protocol/src/profile.rshaya_protocol/src/registry.rshaya_protocol/src/serverbound.rshaya_protocol/src/serverbound/game.rshaya_protocol/src/trim.rshaya_protocol/src/types.rshaya_ser/Cargo.tomlhaya_ser/README.mdhaya_ser/src/lib.rshaya_ser/src/read.rshaya_ser/src/reader.rshaya_ser/src/varint.rshaya_ser/src/write.rshaya_ser_macro/Cargo.tomlhaya_ser_macro/README.mdhaya_ser_macro/src/deserialize.rshaya_ser_macro/src/lib.rshaya_ser_macro/src/serialize.rshaya_str/Cargo.tomlhaya_str/src/lib.rshaya_text/Cargo.tomlhaya_text/src/chat.rshaya_text/src/click_event.rshaya_text/src/color.rshaya_text/src/decoration.rshaya_text/src/dialog.rshaya_text/src/hover_event.rshaya_text/src/json.rshaya_text/src/lib.rshaya_text/src/profile.rsrelease.toml
💤 Files with no reviewable changes (1)
- release.toml
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
haya_text/src/chat.rs (3)
335-351: 🎯 Functional Correctness | 🟡 MinorAccept
Tag::Shortin numeric shadow-color decoding.The
Tagenum inhaya_nbtdefinesShort(i16)as a standard NBT primitive. The currentdeserializeimplementation inhaya_text/src/chat.rshandlesTag::Bytebut omitsTag::Short, causing valid NBT short values to be rejected while narrower byte values are accepted. This creates an inconsistent handling of numeric types.Proposed fix
Tag::Byte(value) => Ok(Self { value: value as u32, }), + Tag::Short(value) => Ok(Self { + value: value as u32, + }), Tag::Long(value) => Ok(Self { value: value as u32, }),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@haya_text/src/chat.rs` around lines 335 - 351, The numeric shadow-color deserialization in deserialize currently accepts Tag::Byte but rejects the standard NBT Tag::Short variant, so update the match in this function to handle Tag::Short the same way as the other numeric cases. Keep the existing conversion to u32 and preserve the current behavior for the other Tag variants in haya_text/src/chat.rs, using deserialize as the location to add the missing case.
537-557: 🎯 Functional Correctness | 🟠 MajorThe current deserialization logic for
ListTagincorrectly rejects valid homogeneous string lists, causing failures when parsingtranslate.withandextrafields that contain string arrays.The
haya_nbtcrate correctly parses string lists asListTag::String, butTextComponent::deserializeinhaya_text/src/chat.rsonly handlesListTag::Compoundat the following locations:
- Lines 537-557:
Tag::Listhandling in the main match- Lines 610-620:
translate.withhandling- Lines 827-836:
extrahandlingThis results in
Errreturns for valid NBT structures. Update the match arms to handleListTag::Stringby converting each string to a literalTextComponent(e.g.,Content::Literal).Required Logic Change
Current (incorrect):
Tag::List(v) => match v { ListTag::Compound(_) => { /* ... */ } // Missing: ListTag::String(_) _ => return Err(...), }Proposed (correct):
Tag::List(v) => match v { ListTag::Compound(list) => { /* existing compound logic */ } ListTag::String(strs) => { // Treat each string as a literal component let mut comp = Self::deserialize(Tag::Compound(/* empty or minimal */))?; // Or construct literal directly strs.into_iter().for_each(|s| { comp.siblings.push(Self::new_literal(s)); // Or equivalent }); Ok(comp) } _ => return Err(...), }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@haya_text/src/chat.rs` around lines 537 - 557, TextComponent::deserialize currently only accepts ListTag::Compound for NBT list fields, so valid homogeneous string arrays in translate.with and extra still fail. Update the Tag::List match in TextComponent::deserialize, plus the translate.with and extra handling branches, to also accept ListTag::String and convert each string entry into a literal TextComponent/Content::Literal sibling instead of returning Err. Keep the existing compound-list behavior unchanged and use the same deserialization path where possible so both compound and string list forms are supported.
425-428: 🩺 Stability & Availability | 🟡 MinorReplace the unsafe
unreachable_unchecked()with a safeunreachable!()macro.
TextComponent::serialize()is guaranteed to returnTag::Compoundbased on its implementation. However, usingunsafe { core::hint::unreachable_unchecked() }introduces undefined behavior (UB) if this invariant ever drifts. Swapping to the safeunreachable!macro maintains correctness while providing a panic with a helpful message instead of UB if the invariant is violated.Proposed fix
for arg in args { vec.push(match arg.serialize() { Tag::Compound(x) => x, - _ => unsafe { core::hint::unreachable_unchecked() }, + _ => unreachable!("TextComponent::serialize must return a compound tag"), }); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@haya_text/src/chat.rs` around lines 425 - 428, The match inside the argument serialization path in TextComponent::serialize currently uses unsafe core::hint::unreachable_unchecked() for the non-Compound case. Replace that unsafe fallback with a safe unreachable!() macro, keeping the same match on arg.serialize() but adding a clear panic message so the invariant is enforced without introducing UB.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@haya_text/src/chat.rs`:
- Around line 335-351: The numeric shadow-color deserialization in deserialize
currently accepts Tag::Byte but rejects the standard NBT Tag::Short variant, so
update the match in this function to handle Tag::Short the same way as the other
numeric cases. Keep the existing conversion to u32 and preserve the current
behavior for the other Tag variants in haya_text/src/chat.rs, using deserialize
as the location to add the missing case.
- Around line 537-557: TextComponent::deserialize currently only accepts
ListTag::Compound for NBT list fields, so valid homogeneous string arrays in
translate.with and extra still fail. Update the Tag::List match in
TextComponent::deserialize, plus the translate.with and extra handling branches,
to also accept ListTag::String and convert each string entry into a literal
TextComponent/Content::Literal sibling instead of returning Err. Keep the
existing compound-list behavior unchanged and use the same deserialization path
where possible so both compound and string list forms are supported.
- Around line 425-428: The match inside the argument serialization path in
TextComponent::serialize currently uses unsafe
core::hint::unreachable_unchecked() for the non-Compound case. Replace that
unsafe fallback with a safe unreachable!() macro, keeping the same match on
arg.serialize() but adding a clear panic message so the invariant is enforced
without introducing UB.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 23dc2b6d-4ad4-4d23-b510-f85f62cce745
📒 Files selected for processing (2)
haya_text/src/chat.rshaya_text/src/decoration.rs
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
haya_nbt/src/stringify.rs (1)
557-559: 🩺 Stability & Availability | 🔴 CriticalCritical Safety Issue: Unsafe Slice Bounds in UTF-8 Encoding
The code at line 558 constructs a mutable slice with an invalid length, causing it to extend beyond allocated memory. Specifically,
core::slice::from_raw_parts_mutis called with a start pointer atbuf.as_mut_ptr().add(buf.len())but a length ofbuf.capacity(). The actual available space from the start pointer is onlybuf.capacity() - buf.len(). Whenbuf.len() > 0, the created slice references memory outside the vector's bounds.Executing
ch.encode_utf8on this oversized slice results in undefined behavior, as it assumes the entire slice length is valid writable memory.Apply the proposed fix to limit the slice length to the exact bytes required (
ch.len_utf8()) or the available tail capacity. This ensures memory safety by strictly adhering to the allocated buffer boundaries.Proposed fix
let ch = escape_quoted(n)?; buf.reserve(ch.len_utf8()); + let len = buf.len(); let s = ch.encode_utf8(unsafe { - core::slice::from_raw_parts_mut(buf.as_mut_ptr().add(buf.len()), buf.capacity()) + core::slice::from_raw_parts_mut(buf.as_mut_ptr().add(len), ch.len_utf8()) }); - unsafe { buf.set_len(buf.len() + s.len()) } + unsafe { buf.set_len(len + s.len()) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@haya_nbt/src/stringify.rs` around lines 557 - 559, The UTF-8 encoding path in stringify uses an unsafe tail slice with an invalid length, so adjust the `ch.encode_utf8` call to only expose the remaining writable space from `buf.as_mut_ptr().add(buf.len())`. Use the exact required byte count (`ch.len_utf8()`) or the vector’s available tail capacity when building the mutable slice with `core::slice::from_raw_parts_mut`, and keep the fix localized to the encoding logic in `stringify.rs` so the slice never extends past the buffer bounds.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@haya_protocol/Cargo.toml`:
- Line 3: The crate version in Cargo.toml is using SemVer build metadata to
encode the protocol revision, which Cargo and crates.io ignore. Update the
version in haya_protocol to a standard SemVer release number by bumping the
patch version for protocol changes, and keep the Minecraft revision in separate
metadata or changelog instead of the version field.
In `@haya_str/src/lib.rs`:
- Around line 148-154: The len-bounded unsafe string conversion is duplicated
between HayaStr::as_mut_str and the AsMut<str> implementation, which risks the
UTF-8 and length invariants drifting apart. Refactor the shared unsafe
construction into one helper on HayaStr and have both as_mut_str and AsMut<str>
delegate to it, keeping the raw slice/from_utf8_unchecked_mut logic in a single
place.
---
Duplicate comments:
In `@haya_nbt/src/stringify.rs`:
- Around line 557-559: The UTF-8 encoding path in stringify uses an unsafe tail
slice with an invalid length, so adjust the `ch.encode_utf8` call to only expose
the remaining writable space from `buf.as_mut_ptr().add(buf.len())`. Use the
exact required byte count (`ch.len_utf8()`) or the vector’s available tail
capacity when building the mutable slice with `core::slice::from_raw_parts_mut`,
and keep the fix localized to the encoding logic in `stringify.rs` so the slice
never extends past the buffer bounds.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0bec9d8c-b6ba-488e-82e2-94ef88c609b4
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
.cargo/config.tomlCargo.tomlhaya_data/build.rshaya_nbt/src/stringify.rshaya_protocol/Cargo.tomlhaya_protocol/src/item_stack.rshaya_ser_macro/src/lib.rshaya_str/src/lib.rshaya_text/src/chat.rs
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
haya_text/src/chat.rs (2)
806-817: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAvoid accepting atlas objects with a placeholder sprite.
This branch creates
ObjectInfo::Atlaswith an empty sprite untilOBJECT_SPRITEis seen. If the sprite key is absent, deserialization succeeds with a placeholder value thatserializewould not intentionally emit. Trackatlas/spriteas pending fields and constructObjectInfo::Atlasonly after both required fields are present.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@haya_text/src/chat.rs` around lines 806 - 817, The OBJECT_ATLAS deserialization path in chat.rs is creating Content::Object/ObjectInfo::Atlas with a placeholder sprite before OBJECT_SPRITE arrives, so update this branch to track atlas and sprite as pending values instead of constructing the object immediately. In the deserialize logic around ObjectInfo::Atlas, only build the final Atlas variant once both fields are present, and ensure missing sprite data causes deserialization to fail rather than defaulting to Identifier::new_const("").unwrap().
619-621: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winValidate discriminator fields instead of discarding them.
TYPE,NBT_SOURCE, andOBJECT_TYPEare emitted byserialize, butdeserializeignores their values. Unsupported or mismatched payloads can be accepted and even fall back to the empty literal default from Lines 611-613. Store these discriminator values and reject unknown or inconsistent combinations after parsing.Also applies to: 752-754, 803-805
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@haya_text/src/chat.rs` around lines 619 - 621, The discriminator handling in deserialize is currently discarding the values for TYPE, NBT_SOURCE, and OBJECT_TYPE, which can let unsupported or mismatched payloads slip through and fall back to the empty-literal default. Update the parsing logic in the relevant deserialize match arms to store these fields on the parsed struct, then validate their combination after parsing and reject unknown, missing, or inconsistent discriminator values instead of ignoring them; apply the same fix in the other deserialize sites that mirror this pattern.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@haya_text/src/chat.rs`:
- Around line 806-817: The OBJECT_ATLAS deserialization path in chat.rs is
creating Content::Object/ObjectInfo::Atlas with a placeholder sprite before
OBJECT_SPRITE arrives, so update this branch to track atlas and sprite as
pending values instead of constructing the object immediately. In the
deserialize logic around ObjectInfo::Atlas, only build the final Atlas variant
once both fields are present, and ensure missing sprite data causes
deserialization to fail rather than defaulting to
Identifier::new_const("").unwrap().
- Around line 619-621: The discriminator handling in deserialize is currently
discarding the values for TYPE, NBT_SOURCE, and OBJECT_TYPE, which can let
unsupported or mismatched payloads slip through and fall back to the
empty-literal default. Update the parsing logic in the relevant deserialize
match arms to store these fields on the parsed struct, then validate their
combination after parsing and reject unknown, missing, or inconsistent
discriminator values instead of ignoring them; apply the same fix in the other
deserialize sites that mirror this pattern.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6358e3c1-230e-4fac-a99b-a9c72e7fe97f
📒 Files selected for processing (2)
haya_ident/src/lib.rshaya_text/src/chat.rs
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
haya_data/build.rs (1)
133-144: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winMake registry name normalization collision-safe.
name.replace('/', "_")maps botha/banda_btoa_b, which can generate duplicate type/impl names. Use an injective escape or add a collision assertion before emitting code.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@haya_data/build.rs` around lines 133 - 144, The registry name normalization in the code generation path can collide because name.replace('/', "_") maps distinct inputs like a/b and a_b to the same identifier. Update the name normalization used before enum_head, impl_codec, impl_name, and impl_common to be injective, or add a collision check/assertion in the generation flow so duplicate generated type/impl names are prevented.haya_str/src/lib.rs (1)
7-7: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winKeep
MAXpublic or bump the major version — this crate is published, so makinghaya_str::MAXprivate breaks downstream imports; either keep the constant public or treat this as a semver-major change.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@haya_str/src/lib.rs` at line 7, The `MAX` constant in `haya_str::lib` is currently private, which breaks downstream users importing `haya_str::MAX`. Update the visibility of `MAX` to remain public in the crate root, or if you intentionally want it private, treat the change as a semver-major API break and adjust the release accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@haya_nbt/src/stringify.rs`:
- Around line 556-561: The character append logic in the string escaping code
uses `from_raw_parts_mut` on Vec spare capacity, which is not safe for
uninitialized bytes. Update the `stringify` path around the `ch.encode_utf8`
call to avoid creating a mutable slice over spare capacity; instead write into a
temporary `[u8; 4]` and append with `extend_from_slice`, or use
`Vec::spare_capacity_mut()` with `MaybeUninit` in the same helper.
In `@haya_text/src/chat.rs`:
- Around line 939-949: The `Content::Object` parsing in `chat.rs` is too
permissive: `atlas` gets ignored when `sprite` is missing, and an
`ObjectInfo::Atlas` can be constructed from malformed payloads that also lack
the proper `object` discriminator. Update the object parsing branch around
`ObjectInfo::Atlas` to validate the field combination first, and return
`Err(Error)` for orphaned `atlas` values or conflicting payloads before
assigning to `content`; use the existing `sprite`, `atlas`, and
`Content::Object`/`ObjectInfo::Atlas` symbols to keep the fix local.
---
Outside diff comments:
In `@haya_data/build.rs`:
- Around line 133-144: The registry name normalization in the code generation
path can collide because name.replace('/', "_") maps distinct inputs like a/b
and a_b to the same identifier. Update the name normalization used before
enum_head, impl_codec, impl_name, and impl_common to be injective, or add a
collision check/assertion in the generation flow so duplicate generated
type/impl names are prevented.
In `@haya_str/src/lib.rs`:
- Line 7: The `MAX` constant in `haya_str::lib` is currently private, which
breaks downstream users importing `haya_str::MAX`. Update the visibility of
`MAX` to remain public in the crate root, or if you intentionally want it
private, treat the change as a semver-major API break and adjust the release
accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2222d055-d774-4d58-bdb0-5bf1658e5a7c
📒 Files selected for processing (4)
haya_data/build.rshaya_nbt/src/stringify.rshaya_str/src/lib.rshaya_text/src/chat.rs
|
@coderabbitai review |
✅ Action performedReview finished.
|
1 participant
Summary by CodeRabbit
New Features
TextComponent, includingClickEventandHoverEvent.TextColorparsing andDecorationMaputilities.Bug Fixes
Breaking Changes
StringTag/CompoundTag, “raw” component/profile representations, redesigned item/dialog/stack structures).Chores