Bug fix: Underscores are not allowed in batch IDs as per the batch spec

[ValwareIRC]

From https://ircv3.net/specs/extensions/batch

The reference tag MUST be treated as an opaque identifier. Reference tag MUST contain only ASCII letters, numbers, and/or hyphen, and MUST be case-sensitive.

Discovered thanks to UnrealIRCd's implementation which correctly told us we're doing it wrong. Means Ergo has a bug.

Summary by CodeRabbit

• Bug Fixes
  • Fixed message segment processing to properly preserve whitespace formatting in message chunks
• Style
  • Updated batch identifier format for improved consistency

[github-actions]

Preview URL: https://batch-fix.obsidianirc.pages.dev

Automated deployment preview for the PR in the Cloudflare Pages.

[coderabbitai]

🧹 Nitpick comments (1)

src/lib/messageProtocol.ts (1)

89-91: Good fix for IRC batch spec compliance.

The change from underscores to hyphens correctly addresses the specification requirement that batch reference tags must only contain ASCII letters, numbers, and hyphens.

One optional cleanup: substr is deprecated. Consider using substring or slice instead.

,

♻️ Optional: Replace deprecated substr

 export const createBatchId = (): string => {
-  return `ml-${Date.now()}-${Math.random().toString(36).substr(2, 9)}`;
+  return `ml-${Date.now()}-${Math.random().toString(36).slice(2, 11)}`;
 };

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/messageProtocol.ts` around lines 89 - 91, The createBatchId function
uses the deprecated String.prototype.substr; update createBatchId to use a
non-deprecated alternative (e.g., String.prototype.slice or
String.prototype.substring) when truncating the Math.random().toString(36)
output so the generated tag still uses the same 9-character portion; keep the
existing template format (`ml-${Date.now()}-${...}`) and hyphen usage unchanged.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/lib/messageProtocol.ts`:
- Around line 89-91: The createBatchId function uses the deprecated
String.prototype.substr; update createBatchId to use a non-deprecated
alternative (e.g., String.prototype.slice or String.prototype.substring) when
truncating the Math.random().toString(36) output so the generated tag still uses
the same 9-character portion; keep the existing template format
(`ml-${Date.now()}-${...}`) and hyphen usage unchanged.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6f48811d-18d0-4253-adaf-10cf1707dc46

📥 Commits

Reviewing files that changed from the base of the PR and between 38c17c2 and 1792354.

📒 Files selected for processing (1)

• src/lib/messageProtocol.ts

[coderabbitai]

📝 Walkthrough

Walkthrough

Modified splitLongMessage to preserve whitespace in completed chunks across three locations. Updated createBatchId to use hyphen-separated format instead of underscore-separated format for batch ID generation. No function signatures were altered.

Changes

Cohort / File(s)	Summary
Message Processing Utilities src/lib/messageProtocol.ts	Removed whitespace trimming from completed line chunks in splitLongMessage function. Changed createBatchId batch ID format from underscore-separated (ml_<timestamp>_<random>) to hyphen-separated (ml-<timestamp>-<random>).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 Hyphens dance where underscores stood,
Whitespace kept as it should!
Messages split with care so fine,
Batch IDs fresh in new design. ✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: replacing underscores with hyphens in batch ID format to comply with IRCv3 batch specification requirements.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch batch-fix

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

src/lib/messageProtocol.ts (1)

90-90: Replace deprecated substr with slice in batch ID generation.

substr is deprecated (Annex B only, non-standard) and should be replaced with slice, which is part of the core ECMAScript spec. The conversion substr(2, 9) → slice(2, 11) is correct.

♻️ Proposed refactor

-export const createBatchId = (): string => {
-  return `ml-${Date.now()}-${Math.random().toString(36).substr(2, 9)}`;
-};
+export const createBatchId = (): string => {
+  return `ml-${Date.now()}-${Math.random().toString(36).slice(2, 11)}`;
+};

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/messageProtocol.ts` at line 90, The batch ID generation uses
deprecated String.prototype.substr; update the expression in the function that
returns the ID (the line returning `ml-${Date.now()}-${...}`) to use slice
instead of substr and adjust the end index accordingly (change `substr(2, 9)` to
`slice(2, 11)`) so the generated ID length remains the same and avoids the
non-standard API.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/lib/messageProtocol.ts`:
- Line 90: The batch ID generation uses deprecated String.prototype.substr;
update the expression in the function that returns the ID (the line returning
`ml-${Date.now()}-${...}`) to use slice instead of substr and adjust the end
index accordingly (change `substr(2, 9)` to `slice(2, 11)`) so the generated ID
length remains the same and avoids the non-standard API.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5e7c62ce-31f5-4b6f-a0cd-7254ba27e6ab

📥 Commits

Reviewing files that changed from the base of the PR and between 38c17c2 and 585d091.

📒 Files selected for processing (1)

• src/lib/messageProtocol.ts

[ValwareIRC]

please merge asap

[anotherdoesnm]

you can't do it?

[ValwareIRC]

you can't do it?

we have a policy for reviewing before merging, and another dev must review it before a pull request can be merged. so no