chore: migrate @rocket.chat/mp3-encoder from Fuselage - #41745
Merged
Conversation
Contributor
|
Looks like this PR is ready to merge! 🎉 |
|
Contributor
WalkthroughThis PR adds the workspace ChangesMP3 encoder package
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant MainThread
participant Worker
participant Mp3Encoder
participant Lame
MainThread->>Worker: init
Worker->>Mp3Encoder: create encoder
MainThread->>Worker: encode
Worker->>Mp3Encoder: encodeBuffer(samples)
Mp3Encoder->>Lame: lame_encode_buffer(...)
MainThread->>Worker: finish
Worker->>Mp3Encoder: flush()
Mp3Encoder->>Lame: lame_encode_flush(...)
Worker->>MainThread: MP3 bytes
sequenceDiagram
participant Mp3Encoder
participant Lame
participant Encoder
participant PsyModel
participant Quantize
participant BitStream
Mp3Encoder->>Lame: lame_encode_buffer(...)
Lame->>Encoder: encode frame
Encoder->>PsyModel: analyze frame
Encoder->>Quantize: quantize frame
Quantize->>BitStream: write coded data
Encoder->>BitStream: format_bitstream(...)
Lame->>Mp3Encoder: output bytes
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
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. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #41745 +/- ##
===========================================
+ Coverage 68.68% 68.93% +0.24%
===========================================
Files 4166 4217 +51
Lines 159382 165752 +6370
Branches 28253 29431 +1178
===========================================
+ Hits 109479 114264 +4785
- Misses 44747 46322 +1575
- Partials 5156 5166 +10
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
![hacktron-app[bot]](https://avatars.githubusercontent.com/in/1250513?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (11)
packages/mp3-encoder/src/lame/math.ts (1)
26-28: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
equalsreturnstruewhen an argument isNaN.
Math.abs(NaN - b) > 0evaluates tofalse, soequalsreturnstruefor anyNaNinput.packages/mp3-encoder/src/lame/ABRPresets.tsandpackages/mp3-encoder/src/lame/VBRPresets.tsuseequals(value, -1)as a "not configured" sentinel test, so aNaNfield would silently pass the sentinel test and get overwritten. The original C comparison is a plain equality test. Usea === bto match it.♻️ Proposed change
export function equals(a: number, b: number) { - return !(Math.abs(a - b) > 0); + return a === b; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/mp3-encoder/src/lame/math.ts` around lines 26 - 28, Update the equals function to use strict numeric equality via a === b, matching the original C comparison and ensuring NaN is not treated as equal to the -1 sentinel.packages/mp3-encoder/src/lame/bitrates.ts (2)
56-59: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
getBitrateignores the sample rate and can select the wrong table.
getBitratecallsgetBitrates(version)without a sample rate, so it never selectsbitratesMap.mpeg2_5. Today the result stays correct becausempeg2andmpeg2_5hold identical values for indices 1 through 8, which are the only valid MPEG 2.5 indices. The function is still fragile if either table changes. Accept the sample rate and forward it, so all three lookups use the same selection rule.The caller in
packages/mp3-encoder/src/lame/BitStream.ts(lines 37-50) passes onlygfp.versionandgfc.bitrate_index, so this change requires passinggfp.out_sampleratethere as well.♻️ Proposed change
-export function getBitrate(version: 0 | 1, index: number): Bitrate | 0 | -1 { - const bitrates = getBitrates(version); +export function getBitrate(version: 0 | 1, index: number, samplerate?: number): Bitrate | 0 | -1 { + const bitrates = getBitrates(version, samplerate); return bitrates[index]; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/mp3-encoder/src/lame/bitrates.ts` around lines 56 - 59, Update getBitrate to accept a sample-rate parameter and forward it to getBitrates so MPEG 2.5 selects the correct table; update the caller in BitStream to pass gfp.out_samplerate alongside gfp.version and gfc.bitrate_index.
9-14: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the declared constant in the comparison.
Line 12 compares
sampleratewith the literal16000, butLOW_SAMPLE_RATE_THRESHOLDholds the same value. The constant is also reused as the default parameter value, which couples two unrelated meanings. Use the constant in the comparison, and give the default parameter its own named constant if the intent differs.♻️ Proposed change
-const getBitrates = (version: 0 | 1, samplerate = LOW_SAMPLE_RATE_THRESHOLD) => { - if (samplerate < 16000) { +const getBitrates = (version: 0 | 1, samplerate = LOW_SAMPLE_RATE_THRESHOLD) => { + if (samplerate < LOW_SAMPLE_RATE_THRESHOLD) { return bitratesMap.mpeg2_5; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/mp3-encoder/src/lame/bitrates.ts` around lines 9 - 14, Update getBitrates to compare samplerate against LOW_SAMPLE_RATE_THRESHOLD instead of the duplicated 16000 literal. If the default samplerate is intended to represent a separate concept, introduce a distinct named constant for that default and use it in the parameter declaration.packages/mp3-encoder/src/lame/VBRTag.ts (1)
27-31: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winOut-of-range reads corrupt the CRC silently.
updateMusicCRCdoes not checkbufferPos + sizeagainstbuffer.length. If the range exceeds the buffer,buffer[bufferPos + i]returnsundefined,crc ^ undefinedevaluates tocrc ^ 0, and the CRC is wrong with no error. Add a bounds check so a caller mistake fails loudly.🛡️ Proposed change
updateMusicCRC(crc: Int32Array, buffer: Uint8Array, bufferPos: number, size: number) { + if (bufferPos < 0 || bufferPos + size > buffer.length) { + throw new RangeError('updateMusicCRC: range out of buffer bounds'); + } + for (let i = 0; i < size; ++i) { crc[0] = this.crcUpdateLookup(buffer[bufferPos + i], crc[0]); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/mp3-encoder/src/lame/VBRTag.ts` around lines 27 - 31, Update updateMusicCRC to validate that bufferPos and size define a range entirely within buffer.length before iterating; reject invalid ranges with an error so no out-of-range byte is read, while preserving the existing CRC update behavior for valid ranges.eslint.config.mjs (1)
476-481: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLimit lint exceptions to the legacy encoder implementation.
The
packages/mp3-encoder/src/**/*.tsglob also matchessrc/index.tsand the package specification files. The override disables both rules for new package glue and tests, not only LAME-compatible identifiers.Scope the override to
packages/mp3-encoder/src/lame/**/*.ts, or use narrow file-level exceptions where required. This keeps lint protection on the public API and tests.Possible scope reduction
- files: ['packages/mp3-encoder/src/**/*.ts'], + files: ['packages/mp3-encoder/src/lame/**/*.ts'],🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@eslint.config.mjs` around lines 476 - 481, Restrict the ESLint override containing `@typescript-eslint/naming-convention` and new-cap in the mp3-encoder configuration to packages/mp3-encoder/src/lame/**/*.ts, or replace it with narrow file-level exceptions only where needed. Keep lint enforcement enabled for src/index.ts, package specification files, and tests.packages/mp3-encoder/src/lame/Encoder.ts (1)
145-150: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the redundant
masking_MSreassignments.Line 145 already fills the 2×2 structure with distinct
III_psy_ratioinstances. Lines 147-150 replace all four of them with new instances. The replacement has no effect on behavior.Delete lines 147-150.
♻️ Proposed cleanup
const masking_MS = Array.from({ length: 2 }, () => Array.from({ length: 2 }, () => new III_psy_ratio())); - masking_MS[0][0] = new III_psy_ratio(); - masking_MS[0][1] = new III_psy_ratio(); - masking_MS[1][0] = new III_psy_ratio(); - masking_MS[1][1] = new III_psy_ratio(); - let masking;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/mp3-encoder/src/lame/Encoder.ts` around lines 145 - 150, Remove the four explicit masking_MS[0][0], masking_MS[0][1], masking_MS[1][0], and masking_MS[1][1] reassignments after the Array.from initialization; retain the existing 2×2 initialization that creates distinct III_psy_ratio instances.packages/mp3-encoder/src/lame/Quantize.ts (1)
41-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDrop the unused
sumparameter frominit_xrpow_core.Line 42 overwrites the
sumparameter with0before any read. The value passed at line 64 has no effect. Remove the parameter and declare the accumulator locally.♻️ Proposed signature cleanup
- private init_xrpow_core(cod_info: GrInfo, xrpow: Float32Array, upper: number, sum: number) { - sum = 0; + private init_xrpow_core(cod_info: GrInfo, xrpow: Float32Array, upper: number) { + let sum = 0; for (let i = 0; i <= upper; ++i) {- sum = this.init_xrpow_core(cod_info, xrpow, upper, sum); + sum = this.init_xrpow_core(cod_info, xrpow, upper);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/mp3-encoder/src/lame/Quantize.ts` around lines 41 - 64, Update Quantize.init_xrpow_core to remove its unused sum parameter, declare the accumulator locally inside the method, and adjust the call in init_xrpow to pass only the remaining required arguments while preserving the returned sum.packages/mp3-encoder/src/lame/Lame.ts (1)
307-322: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCollapse the identical
vbrmodebranches.Lines 318-322 select
CBRNewIterationLoopin both theifand theelsebranch. The condition has no effect.Keep the assignment and drop the branch.
♻️ Proposed simplification
- if (vbrmode === VbrMode.vbr_off) { - gfc.iteration_loop = new CBRNewIterationLoop(this.qu); - } else { - gfc.iteration_loop = new CBRNewIterationLoop(this.qu); - } + gfc.iteration_loop = new CBRNewIterationLoop(this.qu);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/mp3-encoder/src/lame/Lame.ts` around lines 307 - 322, In the iteration-loop setup after the PSY mask assignments, remove the redundant vbrmode conditional and assign a new CBRNewIterationLoop to gfc.iteration_loop directly. Preserve the existing constructor argument this.qu.packages/mp3-encoder/src/lame/Takehiro.ts (1)
399-418: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winWrap the
defaultclause body in a block.Lines 405 and 411 declare
choice2andchoicewithletdirectly in thedefaultclause. Aswitchbody is one block scope, so these bindings are visible to every other clause and sit in the temporal dead zone there. Static analysis reports this as a correctness problem. The standardno-case-declarationslint rule targets the same pattern.No runtime fault occurs today because every earlier clause returns. Add the block to keep the scope explicit and to keep lint clean.
♻️ Proposed scoping fix
- default: + default: { if (max > QuantizePVT.IXMAX_VAL) { s.bits = Takehiro.LARGE_BITS; return -1; } max -= 15; let choice2; for (choice2 = 24; choice2 < 32; choice2++) { if (tables.ht[choice2].linmax >= max) { break; } } let choice; for (choice = choice2 - 8; choice < 24; choice++) { if (tables.ht[choice].linmax >= max) { break; } } return this.count_bit_ESC(ix, ixPos, endPos, choice, choice2, s); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/mp3-encoder/src/lame/Takehiro.ts` around lines 399 - 418, Wrap the body of the switch’s default clause in an explicit block so the let bindings choice2 and choice are scoped only to that clause. Preserve the existing max handling, table searches, and count_bit_ESC return behavior.Source: Linters/SAST tools
packages/mp3-encoder/src/lame/CBRNewIterationLoop.ts (1)
33-45: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the inert
adjustvariable and the inline comment.Two items in this block:
adjustis set to0in both branches and then subtracted, so it never changesmasking_lower_db. The two branches differ only in whether they readmask_adjustormask_adjust_short.- Line 38 holds an implementation comment. The coding guidelines for
**/*.{ts,tsx,js}state "Avoid code comments in the implementation".As per coding guidelines: "Avoid code comments in the implementation".
♻️ Proposed simplification
for (let ch = 0; ch < gfc.channels_out; ch++) { - let adjust; let masking_lower_db; const cod_info = l3_side.tt[gr][ch]; if (cod_info.block_type !== SHORT_TYPE) { - // NORM, START or STOP type - adjust = 0; - masking_lower_db = gfc.PSY.mask_adjust - adjust; + masking_lower_db = gfc.PSY.mask_adjust; } else { - adjust = 0; - masking_lower_db = gfc.PSY.mask_adjust_short - adjust; + masking_lower_db = gfc.PSY.mask_adjust_short; } gfc.masking_lower = Math.pow(10.0, masking_lower_db * 0.1);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/mp3-encoder/src/lame/CBRNewIterationLoop.ts` around lines 33 - 45, In the masking calculation within CBRNewIterationLoop, remove the inert adjust variable and its subtraction, assigning masking_lower_db directly from gfc.PSY.mask_adjust or gfc.PSY.mask_adjust_short in the respective branches. Remove the inline implementation comment while preserving the existing SHORT_TYPE branch behavior.Source: Coding guidelines
packages/mp3-encoder/src/lame/BitStream.ts (1)
639-642: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffReplace the ported C stderr diagnostics with proper error signaling. The encoder runs inside a web worker and writes LAME's original console diagnostics directly to
console.warn. The shared root cause is that Cfprintf(stderr, …)calls were ported verbatim into library code. The caller receives no signal, so a corrupt bit reservoir or an invalid filter configuration is silent to the host application. Line 640-642 also carry unescaped%%sequences from the originalprintfformat strings, which render as a doubled percent sign.
packages/mp3-encoder/src/lame/BitStream.ts#L639-L642: remove the three speculative-cause messages, or fix%%to%and downgrade them to a single message; this block is the most misleading output.packages/mp3-encoder/src/lame/BitStream.ts#L276-L276: surface theMAX_HEADER_BUFoverflow to the caller instead of warning only.packages/mp3-encoder/src/lame/BitStream.ts#L565-L565: return or throw on the negative flush-bit count instead of warning only.packages/mp3-encoder/src/lame/BitStream.ts#L622-L624: report the reservoir inconsistency through the encoder result rather thanconsole.warn.packages/mp3-encoder/src/lame/Lame.ts#L545-L545: report the disabled highpass filter through the initialization result rather thanconsole.warn.packages/mp3-encoder/src/lame/Takehiro.ts#L956-L956: thisdefaultclause is unreachable for the supportedtable_numbervalues; remove the warning or throw.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/mp3-encoder/src/lame/BitStream.ts` around lines 639 - 642, Replace the ported stderr warnings with caller-visible error signaling throughout the encoder. In packages/mp3-encoder/src/lame/BitStream.ts:639-642, remove the speculative diagnostics or consolidate them into one correctly escaped message; at 276, 565, and 622-624, propagate MAX_HEADER_BUF overflow, negative flush-bit counts, and reservoir inconsistency instead of warning only. In packages/mp3-encoder/src/lame/Lame.ts:545, report disabled highpass filtering through initialization results, and in packages/mp3-encoder/src/lame/Takehiro.ts:956, remove the unreachable default warning or make it throw.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/mp3-encoder/CHANGELOG.md`:
- Around line 12-14: Update every release heading in the changelog that precedes
### subsections to use ##, matching the existing heading at the top. Keep ###
headings unchanged as release subsections and preserve all heading text and
links.
In `@packages/mp3-encoder/package.json`:
- Around line 39-50: Add `@rocket.chat/jest-presets` with the workspace:~
specifier and `@rocket.chat/tsconfig` with workspace:* to the package-local
devDependencies alongside the existing Jest and TypeScript tooling.
In `@packages/mp3-encoder/src/lame/arrays.ts`:
- Around line 19-23: Update sortArray to pass a numeric ascending comparator to
the sorted slice’s Array.prototype.sort call, ensuring numeric values are
ordered correctly before being written back into the TypedArray.
In `@packages/mp3-encoder/src/lame/assert.ts`:
- Around line 1-8: Update the assert function to return void without the asserts
condition predicate, since it currently does not throw and must not narrow
caller types such as those in BitStream. Remove the TODO/commented-out assertion
from the implementation, leaving unresolved NaN rationale to documentation or
issue tracking.
In `@packages/mp3-encoder/src/lame/GainAnalysis.ts`:
- Around line 302-329: Update the block counter in the analyzeSamples processing
loop around fsqr and rgData.lsum/rsum to use truncated integer division for
cursamples / 8 before the eight-sample loop. Preserve the existing remainder
pass and ensure the loop executes exactly once per complete eight-sample block.
In `@packages/mp3-encoder/src/lame/Lame.ts`:
- Around line 963-987: Introduce an integer filter midpoint using
Math.trunc(filter_l / 2) in the resampling loop, and reuse it in the boundary
check, j2 calculation, and num_used assignment. Keep the existing filtering
behavior while ensuring num_used remains integral and matches the C filter
indexing.
In `@packages/mp3-encoder/src/lame/Mp3Encoder.spec.ts`:
- Around line 31-40: Update the sample-encoding loops in both fixtures to
process the remaining 126 samples after the maxSamples-sized chunks, using the
corresponding final left and right subarrays with encoder.encodeBuffer and
writing any non-empty output to the hash before calling flush().
In `@packages/mp3-encoder/src/lame/Reservoir.ts`:
- Around line 9-21: Update the Reservoir class field bs to be a readonly
BitStream, matching the constructor’s guaranteed assignment. In ResvFrameBegin,
replace the optional access and ?? 0 fallback with direct getframebits(gfp)
usage so missing streams cannot silently produce an invalid bit budget.
In `@packages/mp3-encoder/src/lame/sampleRates.ts`:
- Around line 3-13: Update findNearestSampleRate to select the supported
SampleRate with the smallest absolute difference from freq, so values just above
a supported rate can resolve downward when appropriate; preserve the existing
supported-rate list and 48000 upper fallback, and add boundary tests covering
nearest-rate choices.
In `@packages/mp3-encoder/src/lame/WavHeader.ts`:
- Around line 31-55: Update the WAV parsing logic in the relevant WavHeader
method to scan RIFF chunks until locating both fmt_ and data, rather than
requiring fmt_ at the initial offset. Advance each chunk by 8 + len + (len & 1),
validate that the chunk header and padded payload remain within
dataView.byteLength, and reject malformed boundaries while preserving the
existing fmt/data parsing behavior.
In `@packages/mp3-encoder/tsconfig.json`:
- Line 5: Update the TypeScript target configured in
packages/mp3-encoder/tsconfig.json so the generated dist output for the static
MP3 worker is transpiled to the supported client browser baseline. Preserve the
direct public-worker loading path, or add the required explicit transpilation
step if the existing target must remain unchanged.
---
Nitpick comments:
In `@eslint.config.mjs`:
- Around line 476-481: Restrict the ESLint override containing
`@typescript-eslint/naming-convention` and new-cap in the mp3-encoder
configuration to packages/mp3-encoder/src/lame/**/*.ts, or replace it with
narrow file-level exceptions only where needed. Keep lint enforcement enabled
for src/index.ts, package specification files, and tests.
In `@packages/mp3-encoder/src/lame/bitrates.ts`:
- Around line 56-59: Update getBitrate to accept a sample-rate parameter and
forward it to getBitrates so MPEG 2.5 selects the correct table; update the
caller in BitStream to pass gfp.out_samplerate alongside gfp.version and
gfc.bitrate_index.
- Around line 9-14: Update getBitrates to compare samplerate against
LOW_SAMPLE_RATE_THRESHOLD instead of the duplicated 16000 literal. If the
default samplerate is intended to represent a separate concept, introduce a
distinct named constant for that default and use it in the parameter
declaration.
In `@packages/mp3-encoder/src/lame/BitStream.ts`:
- Around line 639-642: Replace the ported stderr warnings with caller-visible
error signaling throughout the encoder. In
packages/mp3-encoder/src/lame/BitStream.ts:639-642, remove the speculative
diagnostics or consolidate them into one correctly escaped message; at 276, 565,
and 622-624, propagate MAX_HEADER_BUF overflow, negative flush-bit counts, and
reservoir inconsistency instead of warning only. In
packages/mp3-encoder/src/lame/Lame.ts:545, report disabled highpass filtering
through initialization results, and in
packages/mp3-encoder/src/lame/Takehiro.ts:956, remove the unreachable default
warning or make it throw.
In `@packages/mp3-encoder/src/lame/CBRNewIterationLoop.ts`:
- Around line 33-45: In the masking calculation within CBRNewIterationLoop,
remove the inert adjust variable and its subtraction, assigning masking_lower_db
directly from gfc.PSY.mask_adjust or gfc.PSY.mask_adjust_short in the respective
branches. Remove the inline implementation comment while preserving the existing
SHORT_TYPE branch behavior.
In `@packages/mp3-encoder/src/lame/Encoder.ts`:
- Around line 145-150: Remove the four explicit masking_MS[0][0],
masking_MS[0][1], masking_MS[1][0], and masking_MS[1][1] reassignments after the
Array.from initialization; retain the existing 2×2 initialization that creates
distinct III_psy_ratio instances.
In `@packages/mp3-encoder/src/lame/Lame.ts`:
- Around line 307-322: In the iteration-loop setup after the PSY mask
assignments, remove the redundant vbrmode conditional and assign a new
CBRNewIterationLoop to gfc.iteration_loop directly. Preserve the existing
constructor argument this.qu.
In `@packages/mp3-encoder/src/lame/math.ts`:
- Around line 26-28: Update the equals function to use strict numeric equality
via a === b, matching the original C comparison and ensuring NaN is not treated
as equal to the -1 sentinel.
In `@packages/mp3-encoder/src/lame/Quantize.ts`:
- Around line 41-64: Update Quantize.init_xrpow_core to remove its unused sum
parameter, declare the accumulator locally inside the method, and adjust the
call in init_xrpow to pass only the remaining required arguments while
preserving the returned sum.
In `@packages/mp3-encoder/src/lame/Takehiro.ts`:
- Around line 399-418: Wrap the body of the switch’s default clause in an
explicit block so the let bindings choice2 and choice are scoped only to that
clause. Preserve the existing max handling, table searches, and count_bit_ESC
return behavior.
In `@packages/mp3-encoder/src/lame/VBRTag.ts`:
- Around line 27-31: Update updateMusicCRC to validate that bufferPos and size
define a range entirely within buffer.length before iterating; reject invalid
ranges with an error so no out-of-range byte is read, while preserving the
existing CRC update behavior for valid ranges.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 22aa3dd7-0d5b-444e-a5b8-65229604a105
⛔ Files ignored due to path filters (3)
packages/mp3-encoder/testdata/Left44100.wavis excluded by!**/*.wavpackages/mp3-encoder/testdata/Right44100.wavis excluded by!**/*.wavyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (63)
apps/meteor/package.jsoneslint.config.mjspackages/mp3-encoder/CHANGELOG.mdpackages/mp3-encoder/jest.config.tspackages/mp3-encoder/package.jsonpackages/mp3-encoder/rollup.config.mjspackages/mp3-encoder/src/index.spec.tspackages/mp3-encoder/src/index.tspackages/mp3-encoder/src/lame/ABRPresets.tspackages/mp3-encoder/src/lame/ATH.tspackages/mp3-encoder/src/lame/BitStream.tspackages/mp3-encoder/src/lame/Bits.tspackages/mp3-encoder/src/lame/CBRNewIterationLoop.tspackages/mp3-encoder/src/lame/CalcNoiseData.tspackages/mp3-encoder/src/lame/CalcNoiseResult.tspackages/mp3-encoder/src/lame/Encoder.tspackages/mp3-encoder/src/lame/FFT.tspackages/mp3-encoder/src/lame/GainAnalysis.tspackages/mp3-encoder/src/lame/GrInfo.tspackages/mp3-encoder/src/lame/Header.tspackages/mp3-encoder/src/lame/HuffCodeTab.tspackages/mp3-encoder/src/lame/IIISideInfo.tspackages/mp3-encoder/src/lame/III_psy_ratio.tspackages/mp3-encoder/src/lame/III_psy_xmin.tspackages/mp3-encoder/src/lame/InOut.tspackages/mp3-encoder/src/lame/Lame.tspackages/mp3-encoder/src/lame/LameGlobalFlags.tspackages/mp3-encoder/src/lame/LameInternalFlags.tspackages/mp3-encoder/src/lame/MPEGMode.tspackages/mp3-encoder/src/lame/MeanBits.tspackages/mp3-encoder/src/lame/Mp3Encoder.spec.tspackages/mp3-encoder/src/lame/Mp3Encoder.tspackages/mp3-encoder/src/lame/NewMDCT.tspackages/mp3-encoder/src/lame/NsPsy.tspackages/mp3-encoder/src/lame/NumUsed.tspackages/mp3-encoder/src/lame/PSY.tspackages/mp3-encoder/src/lame/Presets.tspackages/mp3-encoder/src/lame/PsyModel.tspackages/mp3-encoder/src/lame/Quality.tspackages/mp3-encoder/src/lame/Quantize.tspackages/mp3-encoder/src/lame/QuantizePVT.tspackages/mp3-encoder/src/lame/ReplayGain.tspackages/mp3-encoder/src/lame/Reservoir.tspackages/mp3-encoder/src/lame/ScaleFac.tspackages/mp3-encoder/src/lame/ShortBlock.tspackages/mp3-encoder/src/lame/StartLine.tspackages/mp3-encoder/src/lame/Tables.tspackages/mp3-encoder/src/lame/Takehiro.tspackages/mp3-encoder/src/lame/TotalBytes.tspackages/mp3-encoder/src/lame/VBRPresets.tspackages/mp3-encoder/src/lame/VBRTag.tspackages/mp3-encoder/src/lame/VbrMode.tspackages/mp3-encoder/src/lame/WavHeader.tspackages/mp3-encoder/src/lame/arrays.tspackages/mp3-encoder/src/lame/assert.tspackages/mp3-encoder/src/lame/bitrates.tspackages/mp3-encoder/src/lame/constants.tspackages/mp3-encoder/src/lame/getLameShortVersion.tspackages/mp3-encoder/src/lame/index.tspackages/mp3-encoder/src/lame/math.tspackages/mp3-encoder/src/lame/sampleRates.tspackages/mp3-encoder/tsconfig.build.jsonpackages/mp3-encoder/tsconfig.json
📜 Review details
⏰ Context from checks skipped due to timeout. (5)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: 📦 Build Packages
- GitHub Check: Hacktron Security Check
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
packages/mp3-encoder/src/lame/MeanBits.tspackages/mp3-encoder/src/lame/NumUsed.tspackages/mp3-encoder/src/lame/index.tspackages/mp3-encoder/src/lame/TotalBytes.tspackages/mp3-encoder/src/lame/CalcNoiseResult.tspackages/mp3-encoder/src/lame/Header.tspackages/mp3-encoder/src/index.spec.tspackages/mp3-encoder/src/lame/IIISideInfo.tspackages/mp3-encoder/src/lame/III_psy_xmin.tspackages/mp3-encoder/src/lame/InOut.tspackages/mp3-encoder/src/lame/PSY.tspackages/mp3-encoder/src/lame/ATH.tspackages/mp3-encoder/src/lame/MPEGMode.tspackages/mp3-encoder/src/lame/NsPsy.tspackages/mp3-encoder/jest.config.tspackages/mp3-encoder/src/lame/ScaleFac.tspackages/mp3-encoder/src/lame/Bits.tspackages/mp3-encoder/src/lame/Quality.tspackages/mp3-encoder/src/lame/getLameShortVersion.tspackages/mp3-encoder/src/lame/Mp3Encoder.tspackages/mp3-encoder/src/lame/III_psy_ratio.tspackages/mp3-encoder/src/lame/VBRTag.tspackages/mp3-encoder/src/lame/VbrMode.tspackages/mp3-encoder/src/lame/ShortBlock.tspackages/mp3-encoder/src/lame/CalcNoiseData.tspackages/mp3-encoder/src/lame/CBRNewIterationLoop.tspackages/mp3-encoder/src/lame/LameGlobalFlags.tspackages/mp3-encoder/src/lame/arrays.tspackages/mp3-encoder/src/lame/sampleRates.tspackages/mp3-encoder/src/lame/StartLine.tspackages/mp3-encoder/src/lame/ReplayGain.tspackages/mp3-encoder/src/lame/Mp3Encoder.spec.tspackages/mp3-encoder/src/lame/GrInfo.tspackages/mp3-encoder/src/lame/HuffCodeTab.tspackages/mp3-encoder/src/lame/assert.tspackages/mp3-encoder/src/lame/Tables.tspackages/mp3-encoder/src/lame/math.tspackages/mp3-encoder/src/index.tspackages/mp3-encoder/src/lame/Presets.tspackages/mp3-encoder/src/lame/VBRPresets.tspackages/mp3-encoder/src/lame/ABRPresets.tspackages/mp3-encoder/src/lame/constants.tspackages/mp3-encoder/src/lame/Reservoir.tspackages/mp3-encoder/src/lame/Encoder.tspackages/mp3-encoder/src/lame/bitrates.tspackages/mp3-encoder/src/lame/FFT.tspackages/mp3-encoder/src/lame/NewMDCT.tspackages/mp3-encoder/src/lame/LameInternalFlags.tspackages/mp3-encoder/src/lame/Lame.tspackages/mp3-encoder/src/lame/QuantizePVT.tspackages/mp3-encoder/src/lame/Takehiro.tspackages/mp3-encoder/src/lame/PsyModel.tspackages/mp3-encoder/src/lame/GainAnalysis.tspackages/mp3-encoder/src/lame/Quantize.tspackages/mp3-encoder/src/lame/BitStream.tspackages/mp3-encoder/src/lame/WavHeader.ts
packages/**
📄 CodeRabbit inference engine (CLAUDE.md)
Shared libraries belong in
packages/, while other services belong inapps/andee/.
Files:
packages/mp3-encoder/src/lame/MeanBits.tspackages/mp3-encoder/src/lame/NumUsed.tspackages/mp3-encoder/rollup.config.mjspackages/mp3-encoder/src/lame/index.tspackages/mp3-encoder/src/lame/TotalBytes.tspackages/mp3-encoder/src/lame/CalcNoiseResult.tspackages/mp3-encoder/src/lame/Header.tspackages/mp3-encoder/src/index.spec.tspackages/mp3-encoder/src/lame/IIISideInfo.tspackages/mp3-encoder/tsconfig.build.jsonpackages/mp3-encoder/src/lame/III_psy_xmin.tspackages/mp3-encoder/src/lame/InOut.tspackages/mp3-encoder/src/lame/PSY.tspackages/mp3-encoder/src/lame/ATH.tspackages/mp3-encoder/CHANGELOG.mdpackages/mp3-encoder/tsconfig.jsonpackages/mp3-encoder/src/lame/MPEGMode.tspackages/mp3-encoder/src/lame/NsPsy.tspackages/mp3-encoder/jest.config.tspackages/mp3-encoder/src/lame/ScaleFac.tspackages/mp3-encoder/src/lame/Bits.tspackages/mp3-encoder/src/lame/Quality.tspackages/mp3-encoder/src/lame/getLameShortVersion.tspackages/mp3-encoder/src/lame/Mp3Encoder.tspackages/mp3-encoder/src/lame/III_psy_ratio.tspackages/mp3-encoder/src/lame/VBRTag.tspackages/mp3-encoder/package.jsonpackages/mp3-encoder/src/lame/VbrMode.tspackages/mp3-encoder/src/lame/ShortBlock.tspackages/mp3-encoder/src/lame/CalcNoiseData.tspackages/mp3-encoder/src/lame/CBRNewIterationLoop.tspackages/mp3-encoder/src/lame/LameGlobalFlags.tspackages/mp3-encoder/src/lame/arrays.tspackages/mp3-encoder/src/lame/sampleRates.tspackages/mp3-encoder/src/lame/StartLine.tspackages/mp3-encoder/src/lame/ReplayGain.tspackages/mp3-encoder/src/lame/Mp3Encoder.spec.tspackages/mp3-encoder/src/lame/GrInfo.tspackages/mp3-encoder/src/lame/HuffCodeTab.tspackages/mp3-encoder/src/lame/assert.tspackages/mp3-encoder/src/lame/Tables.tspackages/mp3-encoder/src/lame/math.tspackages/mp3-encoder/src/index.tspackages/mp3-encoder/src/lame/Presets.tspackages/mp3-encoder/src/lame/VBRPresets.tspackages/mp3-encoder/src/lame/ABRPresets.tspackages/mp3-encoder/src/lame/constants.tspackages/mp3-encoder/src/lame/Reservoir.tspackages/mp3-encoder/src/lame/Encoder.tspackages/mp3-encoder/src/lame/bitrates.tspackages/mp3-encoder/src/lame/FFT.tspackages/mp3-encoder/src/lame/NewMDCT.tspackages/mp3-encoder/src/lame/LameInternalFlags.tspackages/mp3-encoder/src/lame/Lame.tspackages/mp3-encoder/src/lame/QuantizePVT.tspackages/mp3-encoder/src/lame/Takehiro.tspackages/mp3-encoder/src/lame/PsyModel.tspackages/mp3-encoder/src/lame/GainAnalysis.tspackages/mp3-encoder/src/lame/Quantize.tspackages/mp3-encoder/src/lame/BitStream.tspackages/mp3-encoder/src/lame/WavHeader.ts
apps/meteor/**
📄 CodeRabbit inference engine (CLAUDE.md)
The main Rocket.Chat Meteor application resides in
apps/meteor/; place its application code there rather than in other monorepo areas.
Files:
apps/meteor/package.json
**/*.spec.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.spec.ts: Use descriptive test names that clearly communicate expected behavior in Playwright tests
Use.spec.tsextension for test files (e.g.,login.spec.ts)
Files:
packages/mp3-encoder/src/index.spec.tspackages/mp3-encoder/src/lame/Mp3Encoder.spec.ts
🧠 Learnings (9)
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.
Applied to files:
packages/mp3-encoder/src/lame/MeanBits.tspackages/mp3-encoder/src/lame/NumUsed.tspackages/mp3-encoder/src/lame/index.tspackages/mp3-encoder/src/lame/TotalBytes.tspackages/mp3-encoder/src/lame/CalcNoiseResult.tspackages/mp3-encoder/src/lame/Header.tspackages/mp3-encoder/src/index.spec.tspackages/mp3-encoder/src/lame/IIISideInfo.tspackages/mp3-encoder/src/lame/III_psy_xmin.tspackages/mp3-encoder/src/lame/InOut.tspackages/mp3-encoder/src/lame/PSY.tspackages/mp3-encoder/src/lame/ATH.tspackages/mp3-encoder/src/lame/MPEGMode.tspackages/mp3-encoder/src/lame/NsPsy.tspackages/mp3-encoder/jest.config.tspackages/mp3-encoder/src/lame/ScaleFac.tspackages/mp3-encoder/src/lame/Bits.tspackages/mp3-encoder/src/lame/Quality.tspackages/mp3-encoder/src/lame/getLameShortVersion.tspackages/mp3-encoder/src/lame/Mp3Encoder.tspackages/mp3-encoder/src/lame/III_psy_ratio.tspackages/mp3-encoder/src/lame/VBRTag.tspackages/mp3-encoder/src/lame/VbrMode.tspackages/mp3-encoder/src/lame/ShortBlock.tspackages/mp3-encoder/src/lame/CalcNoiseData.tspackages/mp3-encoder/src/lame/CBRNewIterationLoop.tspackages/mp3-encoder/src/lame/LameGlobalFlags.tspackages/mp3-encoder/src/lame/arrays.tspackages/mp3-encoder/src/lame/sampleRates.tspackages/mp3-encoder/src/lame/StartLine.tspackages/mp3-encoder/src/lame/ReplayGain.tspackages/mp3-encoder/src/lame/Mp3Encoder.spec.tspackages/mp3-encoder/src/lame/GrInfo.tspackages/mp3-encoder/src/lame/HuffCodeTab.tspackages/mp3-encoder/src/lame/assert.tspackages/mp3-encoder/src/lame/Tables.tspackages/mp3-encoder/src/lame/math.tspackages/mp3-encoder/src/index.tspackages/mp3-encoder/src/lame/Presets.tspackages/mp3-encoder/src/lame/VBRPresets.tspackages/mp3-encoder/src/lame/ABRPresets.tspackages/mp3-encoder/src/lame/constants.tspackages/mp3-encoder/src/lame/Reservoir.tspackages/mp3-encoder/src/lame/Encoder.tspackages/mp3-encoder/src/lame/bitrates.tspackages/mp3-encoder/src/lame/FFT.tspackages/mp3-encoder/src/lame/NewMDCT.tspackages/mp3-encoder/src/lame/LameInternalFlags.tspackages/mp3-encoder/src/lame/Lame.tspackages/mp3-encoder/src/lame/QuantizePVT.tspackages/mp3-encoder/src/lame/Takehiro.tspackages/mp3-encoder/src/lame/PsyModel.tspackages/mp3-encoder/src/lame/GainAnalysis.tspackages/mp3-encoder/src/lame/Quantize.tspackages/mp3-encoder/src/lame/BitStream.tspackages/mp3-encoder/src/lame/WavHeader.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.
Applied to files:
packages/mp3-encoder/src/lame/MeanBits.tspackages/mp3-encoder/src/lame/NumUsed.tspackages/mp3-encoder/src/lame/index.tspackages/mp3-encoder/src/lame/TotalBytes.tspackages/mp3-encoder/src/lame/CalcNoiseResult.tspackages/mp3-encoder/src/lame/Header.tspackages/mp3-encoder/src/index.spec.tspackages/mp3-encoder/src/lame/IIISideInfo.tspackages/mp3-encoder/src/lame/III_psy_xmin.tspackages/mp3-encoder/src/lame/InOut.tspackages/mp3-encoder/src/lame/PSY.tspackages/mp3-encoder/src/lame/ATH.tspackages/mp3-encoder/src/lame/MPEGMode.tspackages/mp3-encoder/src/lame/NsPsy.tspackages/mp3-encoder/jest.config.tspackages/mp3-encoder/src/lame/ScaleFac.tspackages/mp3-encoder/src/lame/Bits.tspackages/mp3-encoder/src/lame/Quality.tspackages/mp3-encoder/src/lame/getLameShortVersion.tspackages/mp3-encoder/src/lame/Mp3Encoder.tspackages/mp3-encoder/src/lame/III_psy_ratio.tspackages/mp3-encoder/src/lame/VBRTag.tspackages/mp3-encoder/src/lame/VbrMode.tspackages/mp3-encoder/src/lame/ShortBlock.tspackages/mp3-encoder/src/lame/CalcNoiseData.tspackages/mp3-encoder/src/lame/CBRNewIterationLoop.tspackages/mp3-encoder/src/lame/LameGlobalFlags.tspackages/mp3-encoder/src/lame/arrays.tspackages/mp3-encoder/src/lame/sampleRates.tspackages/mp3-encoder/src/lame/StartLine.tspackages/mp3-encoder/src/lame/ReplayGain.tspackages/mp3-encoder/src/lame/Mp3Encoder.spec.tspackages/mp3-encoder/src/lame/GrInfo.tspackages/mp3-encoder/src/lame/HuffCodeTab.tspackages/mp3-encoder/src/lame/assert.tspackages/mp3-encoder/src/lame/Tables.tspackages/mp3-encoder/src/lame/math.tspackages/mp3-encoder/src/index.tspackages/mp3-encoder/src/lame/Presets.tspackages/mp3-encoder/src/lame/VBRPresets.tspackages/mp3-encoder/src/lame/ABRPresets.tspackages/mp3-encoder/src/lame/constants.tspackages/mp3-encoder/src/lame/Reservoir.tspackages/mp3-encoder/src/lame/Encoder.tspackages/mp3-encoder/src/lame/bitrates.tspackages/mp3-encoder/src/lame/FFT.tspackages/mp3-encoder/src/lame/NewMDCT.tspackages/mp3-encoder/src/lame/LameInternalFlags.tspackages/mp3-encoder/src/lame/Lame.tspackages/mp3-encoder/src/lame/QuantizePVT.tspackages/mp3-encoder/src/lame/Takehiro.tspackages/mp3-encoder/src/lame/PsyModel.tspackages/mp3-encoder/src/lame/GainAnalysis.tspackages/mp3-encoder/src/lame/Quantize.tspackages/mp3-encoder/src/lame/BitStream.tspackages/mp3-encoder/src/lame/WavHeader.ts
📚 Learning: 2026-05-06T12:21:44.083Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 40256
File: apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx:121-149
Timestamp: 2026-05-06T12:21:44.083Z
Learning: Field wrappers in rocket.chat/fuselage-forms (Field, FieldLabel, FieldRow, FieldError, FieldHint) auto-create htmlFor/id associations, aria-describedby, and role="alert" for errors. Do not manually set htmlFor, id, aria-describedby, or role attributes when using these wrappers. This automatic wiring does not apply to plain rocket.chat/fuselage components, which require explicit ID wiring per the accessibility docs. In code reviews, prefer using fuselage-forms wrappers for form fields and verify there is no unnecessary manual ID/aria wiring in files that use these wrappers. If a component uses plain fuselage components, ensure proper id wiring as per docs.
Applied to files:
packages/mp3-encoder/src/lame/MeanBits.tspackages/mp3-encoder/src/lame/NumUsed.tspackages/mp3-encoder/src/lame/index.tspackages/mp3-encoder/src/lame/TotalBytes.tspackages/mp3-encoder/src/lame/CalcNoiseResult.tspackages/mp3-encoder/src/lame/Header.tspackages/mp3-encoder/src/index.spec.tspackages/mp3-encoder/src/lame/IIISideInfo.tspackages/mp3-encoder/src/lame/III_psy_xmin.tspackages/mp3-encoder/src/lame/InOut.tspackages/mp3-encoder/src/lame/PSY.tspackages/mp3-encoder/src/lame/ATH.tspackages/mp3-encoder/src/lame/MPEGMode.tspackages/mp3-encoder/src/lame/NsPsy.tspackages/mp3-encoder/jest.config.tspackages/mp3-encoder/src/lame/ScaleFac.tspackages/mp3-encoder/src/lame/Bits.tspackages/mp3-encoder/src/lame/Quality.tspackages/mp3-encoder/src/lame/getLameShortVersion.tspackages/mp3-encoder/src/lame/Mp3Encoder.tspackages/mp3-encoder/src/lame/III_psy_ratio.tspackages/mp3-encoder/src/lame/VBRTag.tspackages/mp3-encoder/src/lame/VbrMode.tspackages/mp3-encoder/src/lame/ShortBlock.tspackages/mp3-encoder/src/lame/CalcNoiseData.tspackages/mp3-encoder/src/lame/CBRNewIterationLoop.tspackages/mp3-encoder/src/lame/LameGlobalFlags.tspackages/mp3-encoder/src/lame/arrays.tspackages/mp3-encoder/src/lame/sampleRates.tspackages/mp3-encoder/src/lame/StartLine.tspackages/mp3-encoder/src/lame/ReplayGain.tspackages/mp3-encoder/src/lame/Mp3Encoder.spec.tspackages/mp3-encoder/src/lame/GrInfo.tspackages/mp3-encoder/src/lame/HuffCodeTab.tspackages/mp3-encoder/src/lame/assert.tspackages/mp3-encoder/src/lame/Tables.tspackages/mp3-encoder/src/lame/math.tspackages/mp3-encoder/src/index.tspackages/mp3-encoder/src/lame/Presets.tspackages/mp3-encoder/src/lame/VBRPresets.tspackages/mp3-encoder/src/lame/ABRPresets.tspackages/mp3-encoder/src/lame/constants.tspackages/mp3-encoder/src/lame/Reservoir.tspackages/mp3-encoder/src/lame/Encoder.tspackages/mp3-encoder/src/lame/bitrates.tspackages/mp3-encoder/src/lame/FFT.tspackages/mp3-encoder/src/lame/NewMDCT.tspackages/mp3-encoder/src/lame/LameInternalFlags.tspackages/mp3-encoder/src/lame/Lame.tspackages/mp3-encoder/src/lame/QuantizePVT.tspackages/mp3-encoder/src/lame/Takehiro.tspackages/mp3-encoder/src/lame/PsyModel.tspackages/mp3-encoder/src/lame/GainAnalysis.tspackages/mp3-encoder/src/lame/Quantize.tspackages/mp3-encoder/src/lame/BitStream.tspackages/mp3-encoder/src/lame/WavHeader.ts
📚 Learning: 2025-12-10T21:00:43.645Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37091
File: ee/packages/abac/jest.config.ts:4-7
Timestamp: 2025-12-10T21:00:43.645Z
Learning: Adopt the monorepo-wide Jest testMatch pattern: <rootDir>/src/**/*.spec.{ts,js,mjs} (represented here as '**/src/**/*.spec.{ts,js,mjs}') to ensure spec files under any package's src directory are picked up consistently across all packages in the Rocket.Chat monorepo. Apply this pattern in jest.config.ts for all relevant packages to maintain uniform test discovery.
Applied to files:
packages/mp3-encoder/src/index.spec.tspackages/mp3-encoder/src/lame/Mp3Encoder.spec.ts
📚 Learning: 2026-02-24T19:22:48.358Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/omnichannel/omnichannel-send-pdf-transcript.spec.ts:66-67
Timestamp: 2026-02-24T19:22:48.358Z
Learning: In Playwright end-to-end tests (e.g., under apps/meteor/tests/e2e/...), prefer locating elements by translated text (getByText) and ARIA roles (getByRole) over data-qa attributes. If translation values change, update the corresponding test locators accordingly. Never use data-qa locators. This guideline applies to all Playwright e2e test specs in the repository and helps keep tests robust to UI text changes and accessible semantics.
Applied to files:
packages/mp3-encoder/src/index.spec.tspackages/mp3-encoder/src/lame/Mp3Encoder.spec.ts
📚 Learning: 2026-03-06T18:10:15.268Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/code/CodeBlock.spec.tsx:47-68
Timestamp: 2026-03-06T18:10:15.268Z
Learning: In tests (especially those using testing-library/dom/jsdom) for Rocket.Chat components, the HTML <code> element has an implicit ARIA role of 'code'. Therefore, screen.getByRole('code') or screen.findByRole('code') will locate <code> elements even without a role attribute. Do not flag findByRole('code') as invalid in reviews; prefer using the implicit role instead of adding role="code" unless necessary for accessibility.
Applied to files:
packages/mp3-encoder/src/index.spec.tspackages/mp3-encoder/src/lame/Mp3Encoder.spec.ts
📚 Learning: 2026-06-16T14:13:34.463Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 40974
File: packages/web-ui-registration/package.json:31-31
Timestamp: 2026-06-16T14:13:34.463Z
Learning: In Rocket.Chat’s monorepo, when reviewing a dependency entry and flagging that a specific version “does not exist” (e.g., in package.json), first verify the exact package/version directly against the npm registry (use URLs like https://registry.npmjs.org/<package>/<version> or https://www.npmjs.com/package/<package>/v/<version>). Do not rely on web search results for this check, since they may be stale or cached and may not reflect the latest published versions.
Applied to files:
packages/mp3-encoder/package.json
📚 Learning: 2026-06-16T14:13:49.795Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 40974
File: packages/web-ui-registration/package.json:26-26
Timestamp: 2026-06-16T14:13:49.795Z
Learning: During code reviews that check whether a dependency version exists in package.json (especially for Rocket.Chat’s rocket.chat/fuselage and related rocket.chat/fuselage-* packages), don’t rely on web search results. Instead, verify the version directly against the npm registry (e.g., via the npm registry API or the canonical package URL https://www.npmjs.com/package/<package>/v/<version>) before deciding that a version bump is invalid. If the version is present in the npm registry, do not flag it as invalid.
Applied to files:
packages/mp3-encoder/package.json
📚 Learning: 2026-06-16T14:13:59.986Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 40974
File: packages/ui-video-conf/package.json:25-25
Timestamp: 2026-06-16T14:13:59.986Z
Learning: In the Rocket.Chat monorepo, when reviewing a dependency version bump for rocket.chat/fuselage in a package.json, do not flag the new version constraint as “non-existent” or invalid unless you verify the published versions directly from the npm registry (https://www.npmjs.com/package/rocket.chat/fuselage). Don’t rely on search/web results for available versions since they can be stale.
Applied to files:
packages/mp3-encoder/package.json
🪛 ast-grep (0.45.1)
packages/mp3-encoder/src/lame/Mp3Encoder.spec.ts
[warning] 22-22: Avoid SHA1 security protocol
Context: createHash('sha1')
Note: [CWE-327] Use of a Broken or Risky Cryptographic Algorithm (SHA-1).
(avoid-crypto-sha1-typescript)
[warning] 62-62: Avoid SHA1 security protocol
Context: createHash('sha1')
Note: [CWE-327] Use of a Broken or Risky Cryptographic Algorithm (SHA-1).
(avoid-crypto-sha1-typescript)
[warning] 22-22: Do not use weak hash functions (MD5/SHA1)
Context: createHash('sha1')
Note: [CWE-328] Use of Weak Hash.
(insecure-hash-typescript)
[warning] 62-62: Do not use weak hash functions (MD5/SHA1)
Context: createHash('sha1')
Note: [CWE-328] Use of Weak Hash.
(insecure-hash-typescript)
🪛 Biome (2.5.6)
packages/mp3-encoder/src/lame/Takehiro.ts
[error] 405-405: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
(lint/correctness/noSwitchDeclarations)
[error] 411-411: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
(lint/correctness/noSwitchDeclarations)
🪛 markdownlint-cli2 (0.23.2)
packages/mp3-encoder/CHANGELOG.md
[warning] 14-14: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
[warning] 20-20: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
[warning] 30-30: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
[warning] 45-45: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
[warning] 51-51: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
[warning] 57-57: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
[warning] 67-67: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
[warning] 137-137: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🪛 OpenGrep (1.26.0)
packages/mp3-encoder/src/lame/GainAnalysis.ts
[ERROR] 40-40: Possible credit card number (PAN) detected in source code. Credit card numbers should never be hardcoded or stored in source files. Use a secrets manager or tokenization service instead.
(coderabbit.pii.credit-card-number)
[ERROR] 45-45: Possible credit card number (PAN) detected in source code. Credit card numbers should never be hardcoded or stored in source files. Use a secrets manager or tokenization service instead.
(coderabbit.pii.credit-card-number)
[ERROR] 64-64: Possible credit card number (PAN) detected in source code. Credit card numbers should never be hardcoded or stored in source files. Use a secrets manager or tokenization service instead.
(coderabbit.pii.credit-card-number)
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 66 files
Tip: cubic used a learning from your PR history. Let your coding agent read cubic learnings directly with the cubic MCP.
Re-trigger cubic
coderabbitai
Bot
added
type: feature
dougfabris
approved these changes
Aug 11, 2026
gabriellsh
approved these changes
Aug 11, 2026
dougfabris
added
the
stat: QA assured
dionisio-bot
Bot
added
the
stat: ready to merge
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes (including videos or screenshots)
This code was migrated from the Fuselage repository, where
@rocket.chat/mp3-encoderlived until now. The package is a LAME MP3 encoder that runs in a web worker, and it is consumed by a single place in the product — the audio message recorder (AudioEncoder.ts), which loadspublic/workers/mp3-encoder/index.js(a symlink into the package'sdist).Fuselage is a design system repository; an MP3 encoder has nothing to do with a design system, and keeping it there meant every change required a release cycle across two repos before it could reach the product. Moving it here puts the code next to its only consumer.
The migration was done in steps, each one its own commit, so the diff can be reviewed as a sequence rather than as one 11k-line dump:
chore(mp3-encoder): migrate mp3-encoder keeping build output intact— the sources are copied verbatim from Fuselage and the tooling is rewired to this repo's conventions (Rollup,tsconfig, Jest, ESLint). Same entry points, same emitted files, same public types (dist/index.d.tsis byte-identical to the published0.31.26).refactor(mp3-encoder): enforce some lint rules— the code now goes through this repo's ESLint config.@typescript-eslint/naming-conventionandnew-capare disabled forpackages/mp3-encoder/src/**in eslint.config.mjs: the identifiers mirror the original LAME C sources, and renaming them would make the code impossible to check against the reference implementation.test(mp3-encoder): normalize jest configuration— adopts@rocket.chat/jest-presets/client.chore(mp3-encoder): remove unused babel dependencies,chore(mp3-encoder): fix file formatting,chore(mp3-encoder): update URLs— leftovers from the Fuselage setup: dead dev dependencies, formatting, andrepository/bugsURLs still pointing at Fuselage.refactor(meteor): replace mp3-encoder—apps/meteorswitches from@rocket.chat/mp3-encoder: ^0.31.26toworkspace:~.The package keeps its name, its
0.31.26version, itsCHANGELOG.md(Fuselage history included) and itspublishConfig.access: public, so nothing changes for external consumers.No changeset: this does not change behavior for end users.
Issue(s)
ARCH-2358
Steps to test or reproduce
Behavior should be identical to
develop— the point of the migration is that nothing changes for the user.yarn build(oryarn workspace @rocket.chat/mp3-encoder build) so thatpackages/mp3-encoder/distexists and theapps/meteor/public/workers/mp3-encodersymlinks resolve.yarn workspace @rocket.chat/mp3-encoder test— 2 suites, 5 tests, encoding real WAV fixtures undertestdata/.yarn workspace @rocket.chat/mp3-encoder typecheckand... lint— clean (lint reports warnings only, no errors).Further comments
The
distoutput is not committed; it is produced by the package'sbuildscript, andapps/meteor/public/workers/mp3-encoder/{index.js,index.js.map}are symlinks intonode_modules/@rocket.chat/mp3-encoder/dist. That indirection already existed and is untouched — withworkspace:~the symlink now resolves topackages/mp3-encoder/distinstead of a downloaded tarball.One intentional difference from the published artifact is worth calling out for review: the package's
tsconfig.jsonsetstarget: es2024, whereas Fuselage compiled it down toes5. The emitted worker bundle is therefore modern syntax (const, arrow functions, rest parameters) and about 25 KB smaller, but it is not byte-identical to0.31.26. The type surface is unchanged and the encoding logic is the same code; it is only the downlevel emit that went away. Everything Rocket.Chat supports runs this fine, but if we would rather keep the exact published bytes for this release, dropping thetargetoverride is a one-line change.The
src/lamesources remain a close transcription of the LAME reference implementation. They were deliberately left that way instead of being modernized: the value of this code is that it can still be compared line by line with upstream LAME. Any refactor there should be a separate, well-motivated PR.The corresponding package in Fuselage should be deprecated once this lands.
Summary by CodeRabbit
New Features
Tests
Documentation