[klauspost]

Reject unusable inputs to dictionary builder.

Summary by CodeRabbit

• Bug Fixes
  • Dictionary construction now returns errors for invalid size or encoder-level settings and inputs that lack enough usable data.
  • Unreadable sample files are skipped, including when a read returns partial data alongside an error.
  • Added coverage confirming dictionaries can compress and decompress similar samples across multiple encoder levels.

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: dec72da6-3413-4ac8-b9bf-e3bdc576cd7e

📥 Commits

Reviewing files that changed from the base of the PR and between 9abcafb and 1e4f460.

📒 Files selected for processing (5)

• dict/builder.go
• dict/builder_test.go
• dict/cmd/builddict/main.go
• zstd/dict.go
• zstd/dict_test.go

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

---

📝 Walkthrough

Walkthrough

Dictionary construction now rejects invalid sizes, unusable sample sets, and out-of-range Zstandard encoder levels. The builddict command skips files whose reads fail. Tests cover valid dictionary round-trips and invalid inputs.

Changes

Dictionary construction validation

Layer / File(s)	Summary
Builder input checks and round-trip coverage dict/builder.go, dict/builder_test.go, dict/cmd/builddict/main.go	Dictionary construction rejects invalid sizes and sample sets. The builddict command skips files when reads fail. Tests cover successful round-trips and invalid inputs.
Zstandard encoder-level validation zstd/dict.go, zstd/dict_test.go	Zstandard dictionary construction returns an error for encoder levels below speedNotSet or at or above speedLast. Tests check both boundaries.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: lizthegrey

Merge Risk: ⚪ Minimal · up to 1e4f4

The change rejects unusable dictionary inputs and unsupported encoder levels and skips unreadable files. No merge-blocking issue remains; merge after normal checks pass.

Security Architecture Review

Security architecture risk: ⚪ Minimal · up to 1e4f4

The changes reject unusable inputs and invalid compression levels without adding access, privileges, or new execution paths. Failed file reads are excluded from dictionary construction. No material security risk was identified in these changes.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The demonstrated exposure is dictionary construction inside a calling process and the existing local command. No new tenant, service, credential, or data-store access is introduced by the inspected production changes; downstream deployments are not established by this evidence.

Trust Boundaries and Controls

• observed — Caller-provided training data and options still enter through the existing builders. The added checks reject unusable input and invalid levels, while the command no longer includes bytes from failed reads. These are construction controls, not authentication or authorization controls.

Resilience and Maintainability Implications

• observed — The newly rejected paths return no dictionary bytes. Construction uses per-call working state, and the command writes output only after a successful build. The PR does not change the existing output-file write mechanism or its interruption and concurrency behavior.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 15 functions across 5 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: stricter validation for dictionary builder inputs and encoder levels.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

---

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.}