Skip to content

Use strtr#298

Merged
rmcdaniel merged 1 commit intomasterfrom
fix/yenc
Dec 7, 2025
Merged

Use strtr#298
rmcdaniel merged 1 commit intomasterfrom
fix/yenc

Conversation

@rmcdaniel
Copy link
Copy Markdown
Member

This PR simplifies and optimizes the Y serializer’s byte-escaping logic by replacing manual loops with a strtr-based implementation.

@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 7, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (d7b0a17) to head (7e67464).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff             @@
##              master      #298   +/-   ##
===========================================
  Coverage     100.00%   100.00%           
+ Complexity       344       338    -6     
===========================================
  Files             48        48           
  Lines           1420      1413    -7     
===========================================
- Hits            1420      1413    -7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the Y serializer's byte-escaping logic by replacing manual character-by-character loop processing with PHP's native strtr() function, resulting in cleaner, more performant code while maintaining identical behavior.

  • Simplified encode() method using strtr() with a translation table for null and escape bytes
  • Simplified decode() method using strtr() with a translation table for escaped sequences
  • Reduced code complexity from ~30 lines to ~8 lines while preserving the same escape/unescape logic

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@rmcdaniel rmcdaniel merged commit 87fa282 into master Dec 7, 2025
9 of 10 checks passed
@rmcdaniel rmcdaniel deleted the fix/yenc branch December 7, 2025 01:59
rmcdaniel pushed a commit that referenced this pull request Apr 16, 2026
Closes #298. The workflow package's WorkerProtocolVersion constant and
the standalone server's history_page_size_default config had drifted:
the package constant read 200 while the server advertises 500 in its
server_capabilities response. The server is the authority on pagination
size, so external workers already receive 500 at runtime; the constant
was creating a documentation mismatch for SDK authors who referenced it.

The package constant is the contract default that external workers and
SDK authors should assume when the server has not advertised a value.
Updated the docblock to name the authoritative server config key so
future drift is obvious.

14 WorkerProtocolVersion tests (49 assertions) still pass — they
reference the constant symbolically, not the literal value.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants