Skip to content

fix(llm): count escalation payload_chars as characters, not bytes - #852

Merged
matedev01 merged 2 commits into
GeniePod:mainfrom
jeffrey701:fix/escalation-payload-chars
Jul 24, 2026
Merged

fix(llm): count escalation payload_chars as characters, not bytes#852
matedev01 merged 2 commits into
GeniePod:mainfrom
jeffrey701:fix/escalation-payload-chars

Conversation

@jeffrey701

@jeffrey701 jeffrey701 commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Summary

summarize_messages fills EscalationPayloadSummary.payload_chars with String::len() — the UTF-8 byte length — not the character count the field name (and the rest of the codebase) means:

payload_chars: messages.iter().map(|m| m.content.len()).sum(),

So any non-ASCII content over-reports: "café"5 (bytes) instead of 4 (chars); an accented household name ("José"), a non-Latin script, or an emoji device alias — all ordinary for a home assistant — inflate the count. This value isn't cosmetic: server.rs feeds summary.payload_chars into security::audit::log_provider_escalation, so the wrong number is written to the operator audit trail for "data left the device via the gated cloud path" (#570).

The codebase has a clear, tested convention that *_chars metrics are Unicode char counts — tools/dispatch/mod.rs uses .chars().count(), and telegram.rs carries a regression test literally named voice_reply_gate_char_count_uses_unicode_chars_not_bytes. escalation.rs was the lone *_chars site still using .len(). This aligns it; the existing summarize_messages_counts_chars_not_content test only used ASCII (where bytes == chars), so the bug was latent.

Closes #851

Real Behavior Proof

  • I have built and run the affected code locally (or noted why I could not).
  • I have verified the change end-to-end on Jetson hardware.
  • I have NOT verified on Jetson hardware, and I explain the equivalent verification path or validation gap below.

Tested profile / hardware (check all that apply):

  • jetson
  • raspberry_pi
  • portable_sbc
  • laptop
  • mac
  • CI-only / docs-only
  • Not run locally

genie-core is a Linux-only crate, so it doesn't compile on my Windows dev box — CI runs the real cargo test on Linux, exercising the added summarize_counts_unicode_chars_not_bytes test. Locally I ran the old .len() vs new .chars().count() counting side by side in a rustc harness.

What I ran

rustc --edition 2021 pr_chars_test.rs && ./pr_chars_test

What I observed

OK: OLD counted bytes (café=5); NEW counts chars (café=4); ASCII unchanged (=6).

The harness asserts OLD payload_chars("café") == 5 (byte length — the bug), NEW == 4 (char count), the same for "José" (5→4) and an emoji alias (9→6), and that ASCII content is unchanged ("abcd"+"ef" == 6, matching the existing test).

Summary by CodeRabbit

  • Bug Fixes
    • Corrected escalation audit summaries to count Unicode characters accurately instead of counting encoded bytes.
    • Added coverage for messages containing accented and other multi-byte characters.

summarize_messages set EscalationPayloadSummary.payload_chars from String::len()
(UTF-8 byte length) rather than the character count the field name means, so any
non-ASCII content over-reported ("café" -> 5 instead of 4). server.rs feeds this
into security::audit::log_provider_escalation, so the inflated number was written
to the operator audit trail for the gated cloud-escalation path.

The codebase already treats *_chars metrics as Unicode char counts (dispatch uses
chars().count(); telegram has a bytes-vs-chars regression test); escalation.rs was
the lone *_chars site on len(). Count with chars().count(); the existing test only
used ASCII so the bug was latent. Adds a Unicode regression test.
@github-actions github-actions Bot added the bug Something isn't working label Jul 24, 2026
@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 6fd4bc76-0e6f-43d1-a650-9a380f43f5fd

📥 Commits

Reviewing files that changed from the base of the PR and between 7c67906 and cdd3224.

📒 Files selected for processing (1)
  • crates/genie-core/src/llm/escalation.rs

📝 Walkthrough

Walkthrough

The escalation payload summary now counts Unicode characters instead of UTF-8 bytes, and a unit test verifies that multi-byte text such as "café" is counted correctly.

Changes

Unicode payload counting

Layer / File(s) Summary
Payload summary and validation
crates/genie-core/src/llm/escalation.rs
payload_chars now sums content.chars().count(), with a test confirming "café" counts as four characters.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: kiannidev

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and accurately summarizes the main change to count escalation payload characters instead of bytes.
Linked Issues check ✅ Passed The PR implements #851 by switching to Unicode character counts and adding regression coverage for non-ASCII content.
Out of Scope Changes check ✅ Passed The changes stay focused on the payload character-count fix and its test coverage, with no unrelated edits.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@matedev01 matedev01 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — payload_chars used String::len() (UTF-8 byte length) instead of char count, so non-ASCII content over-reported in the escalation audit trail ('café' -> 5 instead of 4). Small, correct fix to the audit-log accuracy from #648/#796. Verified: clippy -D warnings clean, escalation tests (4) pass, fmt clean.

@matedev01
matedev01 merged commit 03c59c9 into GeniePod:main Jul 24, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Escalation audit metric payload_chars counts bytes, not characters (over-reports non-ASCII content)

2 participants