flate: Use exact sizes for huffman tables by klauspost · Pull Request #1103 · klauspost/compress (original) (raw)
📝 Walkthrough
Walkthrough
The code changes adjust a boolean argument in flate/huffman_bit_writer.go’s writeBlockDynamic call to indexTokens, using fillReuse && !sync instead of !sync. Two binary test data files under flate/testdata are replaced with new expected outputs. No exported/public declarations are modified.
Changes
| Cohort / File(s) | Summary of Changes |
|---|---|
| Huffman writer logicflate/huffman_bit_writer.go | Updated argument passed to indexTokens from !sync to fillReuse && !sync; fillReuse is a constant false, so effective behavior remains unchanged. |
| **Test expectations (dynamic Huffman)**flate/testdata/huffman-text.dyn.expect, flate/testdata/huffman-text.dyn.expect-noinput | Replaced binary expected outputs for tests; no code changes. |
Estimated code review effort
🎯 2 (Simple) | ⏱️ ~10 minutes
Pre-merge checks and finishing touches
✅ 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 "flate: Use exact sizes for huffman tables" is concise, single-sentence, and accurately captures the primary change—adjusting Huffman table sizing—which matches the PR objectives and the provided diffs that focus on table-size calculation and related test updates. It avoids noise and is specific enough for a teammate scanning history to understand the main intent. |
| Docstring Coverage | ✅ Passed | No functions found in the changes. Docstring coverage check skipped. |
✨ Finishing touches
- 📝 Generate Docstrings 🧪 Generate unit tests
- Create PR with unit tests
- Post copyable unit tests in a comment
- Commit unit tests in branch
flate-dont-fill
📜 Recent review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between bda824b and 33f1a3f.
📒 Files selected for processing (3)
flate/huffman_bit_writer.go(1 hunks)flate/testdata/huffman-text.dyn.expect(1 hunks)flate/testdata/huffman-text.dyn.expect-noinput(1 hunks) ⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)- GitHub Check: build (1.25.x, ubuntu-latest)
- GitHub Check: build (1.24.x, ubuntu-latest)
- GitHub Check: build (1.25.x, macos-latest)
- GitHub Check: build (1.25.x, windows-latest)
- GitHub Check: build (1.24.x, macos-latest)
- GitHub Check: build (1.23.x, ubuntu-latest)
- GitHub Check: build (1.23.x, macos-latest)
- GitHub Check: build (1.24.x, windows-latest)
- GitHub Check: build (1.23.x, windows-latest)
- GitHub Check: build-special 🔇 Additional comments (3)
flate/testdata/huffman-text.dyn.expect (1)
1-1: Goldens updated — consistent with smaller dynamic headers.These changes align with using exact code counts. Please keep a note in contributor docs (if any) about how to regenerate these fixtures.
If helpful, I can add a short “how to regenerate flate/testdata” doc or script. Want me to open a follow-up?
flate/testdata/huffman-text.dyn.expect-noinput (1)
1-1: Goldens updated — confirm regen source and reproducible size delta (-2 bytes).
- New sizes: flate/testdata/huffman-text.dyn.expect = 238 bytes; flate/testdata/huffman-text.dyn.expect-noinput = 238 bytes.
- Previous (HEAD^): both 240 bytes → delta = -2 bytes each.
Confirm these were regenerated from the updated encoder with exact table sizes and that the outputs are reproducible across platforms.
flate/huffman_bit_writer.go (1)
649-649: Approve — call sites consistent.Calls found: flate/huffman_bit_writer.go:565 — w.indexTokens(tokens, false); 649 — w.indexTokens(tokens, fillReuse && !sync); 723 — w.indexTokens(tokens, true) (inside if fillReuse && !sync). Matches PR intent to avoid over-reporting HLIT/HDIST; no changes required.
Tip
👮 Agentic pre-merge checks are now available in preview!
Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
- Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
- Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.
Please see the documentation for more information.
Example:
reviews: pre_merge_checks: custom_checks: - name: "Undocumented Breaking Changes" mode: "warning" instructions: | Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).
Please share your feedback with us on this Discord post.
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
Comment @coderabbitai help to get the list of available commands and usage tips.