🧪 Unit tests for UTF8Util::decodeSafe and build fixes

[segin]

🎯 What: Implemented unit tests for UTF8Util::decodeSafe and fixed standalone compilation issues for UTF8Util and Rect.
📊 Coverage:

• UTF8Util::decodeSafe: Tested null/empty inputs, valid ASCII/UTF-8, null terminator handling, and invalid sequence repair (including overlong encodings).
• Standalone compilation: Fixed missing header inclusions in UTF8Util.cpp, rect.h, and rect.cpp, allowing these core components to be compiled and tested without requiring a full environment with SDL and TagLib.
  ✨ Result: Improved test coverage for core utility functions and enhanced build robustness for local development and testing.

---

PR created automatically by Jules for task 2525939456556117283 started by @segin

[google-labs-jules]

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.

---

For security, I will only act on instructions from the user who triggered this task.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[github-actions]

Claude finished @segin's task —— View job

---

Code Review Complete

I've thoroughly reviewed this PR which adds unit tests for UTF8Util::decodeSafe and fixes standalone compilation issues. Here's my comprehensive assessment:

✅ Strengths

Excellent Test Coverage for UTF8Util::decodeSafe

The new UTF8DecodeSafeTest class in tests/test_utf8util_unit.cpp provides comprehensive coverage:

• Edge cases: Null/empty inputs, null terminators within buffers
• Valid data: ASCII and UTF-8 with/without null terminators
• Invalid data: Invalid start bytes, overlong encodings
• Repair functionality: Verifies malformed UTF-8 is replaced with replacement characters (U+FFFD)
• Real-world scenarios: Tests realistic input conditions with proper boundary handling

Lines 272-304: The test cases are well-structured and cover critical scenarios that decodeSafe would encounter in production.

Proper Standalone Compilation Fixes

The compilation fixes enable core utilities to be built/tested independently:

• UTF8Util.cpp:13: Added missing core/utility/UTF8Util.h include
• rect.h:103-105: Added essential headers (<cstdint>, <utility>, <string>)
• rect.cpp:28-32: Added required includes for standalone compilation

These changes follow good practices for modular C++ development.

Appropriate CI Infrastructure Updates

The workflow updates are maintenance improvements:

• Upgraded GitHub Actions to latest versions (v3→v4, v4→v5, v6→v7)
• Fixed Claude integration permissions (read→write for PR/issue operations)
• Added missing github_token parameter

🔍 Code Quality Assessment

Test Implementation Quality

tests/test_utf8util_unit.cpp:272-304:

• ✅ Comprehensive edge case coverage
• ✅ Clear test descriptions and expectations
• ✅ Proper use of test framework assertions
• ✅ Realistic test data (overlong encodings, invalid bytes)

Production Code Quality

The UTF8Util::decodeSafe implementation in src/core/utility/UTF8Util.cpp:172-190:

• ✅ Proper null pointer checks
• ✅ Null terminator handling via loop scan
• ✅ Safe UTF-8 validation and repair
• ✅ Consistent with other UTF8Util methods

🚨 Minor Issues Identified

1. Potential Signed/Unsigned Comparison

rect.cpp:802-817: In the normalize() method, there are casts from uint16_t to int32_t for negative checking:

if (static_cast<int32_t>(m_width) < 0) // Line 802

This could be problematic since uint16_t values cannot be negative. Consider checking the intent or adding documentation.

2. Performance Consideration

UTF8Util.cpp:178-181: The null terminator search uses a simple loop:

while (len < size && data[len] != 0x00) {
    ++len;
}

For very large buffers, consider using memchr() for better performance, though the current implementation is safer and more readable.

🔒 Security Assessment

✅ No security concerns identified:

• Proper bounds checking in UTF-8 processing
• Safe null pointer handling
• No buffer overruns in test code
• Input validation follows secure practices

⚡ Performance Impact

✅ Minimal performance impact:

• New tests don't affect runtime performance
• Compilation fixes are include-only changes
• UTF8Util implementation maintains existing performance characteristics

📊 Test Coverage Impact

✅ Significantly improved test coverage:

• Adds comprehensive testing for previously untested decodeSafe method
• Tests critical UTF-8 edge cases (overlong encodings, invalid sequences)
• Validates both the happy path and error handling paths

🎯 Recommendation

APPROVE - This is a solid contribution that:

1. Adds valuable test coverage for a critical utility function
2. Improves build system modularity with proper compilation fixes
3. Updates CI infrastructure appropriately
4. Follows project conventions and quality standards
5. Introduces no security risks or significant performance concerns

The minor issues identified are not blocking and can be addressed in future maintenance if needed.

---

Job Run

[github-actions]

Claude encountered an error —— View job

---

I'll analyze this and get back to you.

[github-actions]

Claude encountered an error —— View job

---

I'll analyze this and get back to you.

[github-actions]

Claude encountered an error —— View job

---

I'll analyze this and get back to you.

[github-actions]

Claude encountered an error —— View job

---

I'll analyze this and get back to you.