fix(skills): preserve emoji in long history scans#107720
Conversation
|
Exact-head CI refresh after the ordinary main sync:
This supersedes the initial red CI classification in the PR body: after syncing the upstream Bonjour and max-lines fixes, the same PR patch is fully green. |
|
@clawsweeper re-review The exact-head CI and proof have been refreshed above. Please review the current head. |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Codex review: needs maintainer review before merge. Reviewed July 15, 2026, 10:07 PM ET / July 16, 2026, 02:07 UTC. Summary PR surface: Source +1, Tests +31. Total +32 across 2 files. Reproducibility: yes. —the raw prefix/head/tail slicing can deterministically split an emoji by positioning its surrogate pair across the configured cap boundary; the supplied production-path run verifies the repaired output, although an unpatched current-main live run was not executed in this review. Review metrics: none identified. Stored data model Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the shared UTF-16 helper implementation after refreshing it against current main, retaining the existing cap, omission marker, redaction order, head/tail layout, and focused boundary tests. Do we have a high-confidence way to reproduce the issue? Yes—the raw prefix/head/tail slicing can deterministically split an emoji by positioning its surrogate pair across the configured cap boundary; the supplied production-path run verifies the repaired output, although an unpatched current-main live run was not executed in this review. Is this the best way to solve the issue? Yes—the PR applies the repository’s existing UTF-16-safe normalization helpers at the actual downstream cap instead of adding a parallel truncation implementation, while preserving the established budget and layout. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 154d53c4f6fc. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +1, Tests +31. Total +32 across 2 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (3 earlier review cycles) |
|
Exact-head real behavior proof is now available for the provider-bound history-scan path.
Machine result: {"proof":"persisted-history-scan-provider-boundary","prHead":"1ba1110785b7cf5b482f268caf10fbf7723229c9","persistedMessages":13,"sourceBoundaryIndex":1287,"sourceBoundaryCodeUnits":["d83d","de00"],"providerRequests":2,"firstRequestPath":"/v1/responses","providerPromptChars":4485,"omissionPresent":true,"danglingSurrogate":false,"completionRoundTrip":true,"candidateCount":1,"reviewedSessions":1,"reviewIdeas":0,"completedIdeas":0}@clawsweeper re-review |
|
Merged via squash.
|
What Problem This Solves
Fixes an issue where Skill Workshop history scanning could produce malformed provider-bound transcript text when its per-session cap split an emoji or another supplementary character in the retained prefix, head, or tail.
Why This Change Was Made
The downstream history-scan cap now uses the normalization core's UTF-16-safe prefix and slice helpers while preserving the existing budget, omission marker, redaction order, and head/tail layout. This is distinct from #106370, which addresses the earlier 60,000-character experience-review formatter cap; this change covers the later 16,000-character per-session history-scan cap.
User Impact
Skill history scans now send well-formed Unicode transcripts at every truncation boundary without increasing their context budget.
Evidence
node scripts/run-vitest.mjs src/skills/workshop/history-scan.test.ts -t 'keeps .* UTF-16 safe'- 3/3 tests passed; 23 skipped.git diff --check origin/main...HEAD- passed.