Skip to content

flate: Writer L7-9 with dictionary can emit the dictionary to stream - #1201

Merged
klauspost merged 1 commit into
masterfrom
flate-dict
Sep 2, 2026
Merged

klauspost merged 1 commit into
masterfrom
flate-dict

Conversation

@klauspost

@klauspost klauspost commented Sep 2, 2026

Copy link
Copy Markdown
Owner

Backport of golang/go#80538

Adds dictionary fuzz check. Status:

fuzz: elapsed: 2h12m45s, execs: 98125523 (22332/sec), new interesting: 152 (total: 3769)
fuzz: elapsed: 2h12m48s, execs: 98203921 (26130/sec), new interesting: 152 (total: 3769)

Summary by CodeRabbit

  • Bug Fixes

    • Corrected block positioning after dictionary data is loaded, improving compression stream generation and round-trip reliability.
  • Tests

    • Added coverage for dictionary-based compression involving non-compressed blocks.
    • Expanded fuzz testing across compression levels, reset states, and matching or non-matching dictionaries.
    • Added an option to run dictionary-focused fuzz tests with a capped input size.

…output stream

Backport of golang/go#80538

Adds dictionary fuzz check. Status:

```
fuzz: elapsed: 2h12m45s, execs: 98125523 (22332/sec), new interesting: 152 (total: 3769)
fuzz: elapsed: 2h12m48s, execs: 98203921 (26130/sec), new interesting: 152 (total: 3769)
```
@coderabbitai

coderabbitai Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Team

Run ID: 5316ca75-c33b-4e2c-afbc-cfda1a94e712

📥 Commits

Reviewing files that changed from the base of the PR and between f93f23a and 010181b.

📒 Files selected for processing (3)
  • flate/deflate.go
  • flate/fuzz_test.go
  • flate/writer_test.go

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


📝 Walkthrough

Walkthrough

The change updates fillWindow block tracking after dictionary data is copied. It adds dictionary-focused fuzzing and regression coverage for non-compressed blocks across compression levels and reset states.

Changes

Dictionary encoding validation

Layer / File(s) Summary
Window boundary tracking
flate/deflate.go
fillWindow sets d.blockStart to the updated d.windowEnd after dictionary data is copied.
Dictionary regression coverage
flate/fuzz_test.go, flate/writer_test.go
Dictionary fuzzing covers two dictionaries, all compression levels, and both reset states. Tests verify non-compressed block round trips and complete stream consumption.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 01018

This localized change corrects dictionary handling in flate and adds regression and fuzz coverage; no actionable merge-blocking risk remains beyond normal checks and review.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 3 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 identifies the main change: flate writers at levels 7–9 can emit a configured dictionary to the stream.
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 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch flate-dict

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.

@klauspost
klauspost merged commit 9d8ccb1 into master Sep 2, 2026
29 checks passed
@klauspost
klauspost deleted the flate-dict branch September 2, 2026 12:08
nadaverell added a commit to skyhook-io/radar that referenced this pull request Sep 9, 2026
Replaces the ten open weekly Dependabot PRs with one reviewed, tested
set. All ten were included; nothing was held or excluded.

## Screening

- **Screening time:** `2026-09-09T06:44:31Z`
- **72-hour cutoff:** a version must have been published before
`2026-09-06T06:44:31Z`

Soak is measured from **upstream publication time** (npm registry `time`
metadata; `proxy.golang.org` `.info` metadata), never from Dependabot PR
creation — all ten PRs were opened ~2h before screening and would fail a
naive PR-age check. The gate was re-applied to the **final resolved
graph**, including every transitive addition and upgrade.

## Included

| PR | Dependency | Old → New | Published (UTC) | Soak | Risk conclusion
|
|---|---|---|---|---|---|
| #1697 | `github.com/klauspost/compress` | 1.19.2 → 1.20.0 |
2026-09-02T12:08:18Z | 6d 18h | Radar imports only `compress/gzip`
([`internal/server/compress.go`](internal/server/compress.go)). Release
is additive (lzw, XPRESS, arm64 asm) plus
[#1201](klauspost/compress#1201), a
**correctness fix** backporting
[golang/go#80538](golang/go#80538) for flate
L7–9 dictionary streams. Radar uses no dictionaries; `gzhttp` jitter
change is in a package Radar doesn't import. |
| #1698 | `modernc.org/sqlite` | 1.57.0 → 1.58.0 | 2026-09-01T15:17:17Z
| 7d 15h | [SQLite 3.53.4](https://sqlite.org/releaselog/3_53_4.html);
upstream's own journal-rollback corruption fix replaces the local
super-journal patch, recovery behavior unchanged. New OFD locking is
**opt-in and off by default** — Radar sets neither
`MODERNC_SQLITE_OFD_LOCK` nor `OFDLocking()`, so locking is
byte-for-byte unchanged. `isCorruptedSQLiteError` in
[`internal/timeline/manager.go`](internal/timeline/manager.go) matches
SQLite core result text; its tests pass. |
| #1699 | `golang.org/x/sys` | 0.47.0 → 0.48.0 | 2026-08-31T19:43:43Z |
8d 11h | Syscall constants/types and cpu detection only. Radar's use is
`windows` (localterm, process) and `unix` (birthtime). Verified by
cross-compiling windows/linux/darwin. |
| #1700 | `github.com/coreos/go-oidc/v3` | 3.20.0 → 3.21.0 |
2026-09-01T23:03:42Z | 7d 7h | Single change:
[#499](coreos/go-oidc#499) ignores JWKs with
unsupported key types instead of failing the whole key set. Tolerance
improvement, not a verification weakening — a supported key is still
required to verify signatures. Auth surface is
[`internal/auth/oidc.go`](internal/auth/oidc.go); `internal/auth` tests
pass. |
| #1701 | `github.com/prometheus/common` | 0.70.1 → 0.71.0 |
2026-08-31T08:47:38Z | 8d 21h | Release is mostly OpenMetrics 2.0
encoder/negotiation work. Radar touches neither: it calls only
`expfmt.NewTextParser(model.LegacyValidation)`
([`internal/upgrade/collectors_live.go`](internal/upgrade/collectors_live.go))
and `model.ParseDuration`
([`internal/mcp/tools_prometheus.go`](internal/mcp/tools_prometheus.go)).
`model` changes are additive/perf. |
| #1702 | `postcss` | 8.5.26 → 8.5.28 | 2026-09-03T15:13:59Z | 5d 15h |
8.5.27 fixes comment handling for `/*#`, `*` hack in custom properties,
and `list.comma()`/`list.space()` edge cases; 8.5.28 fixes the types
regression 8.5.27 introduced (so landing on .28, not .27, matters). CSS
output verified via `make build` + visual test. |
| #1703 | `@tanstack/react-query` | 5.101.4 → 5.102.8 |
2026-08-27T16:06:57Z | 12d 14h | Widest range here. 5.102.0 is a minor
with real **removals** — the `promise` property on query results and
render-time prefetching
([#11221](TanStack/query#11221)),
`experimental_beforeQuery`/`experimental_afterQuery`
([#11233](TanStack/query#11233)), and
`placeholderData` on suspense infinite queries
([#11144](TanStack/query#11144)). Grepped Radar
for each: **zero call sites** for all of them, and no suspense/infinite
query hooks at all. `placeholderData` is used only with plain
`useQuery`. Type changes covered by a clean `make tsc`. |
| #1704 | `react-router-dom` | 7.18.2 → 7.18.3 | 2026-08-28T14:30:10Z |
11d 16h |
[v7.18.3](https://github.com/remix-run/react-router/blob/v7/CHANGELOG.md#v7183):
route-matching perf plus two hardening changes.
[#15446](remix-run/react-router#15446) makes
programmatic navigations **reject external destinations** — checked all
70 `navigate()` call sites in `web/src` and `packages/k8s-ui/src`; every
one passes a relative path or a `{ pathname, search }` object, none an
external URL. |
| #1705 | `react-virtuoso` | 4.18.12 → 4.18.13 | 2026-09-05T14:29:31Z |
**3d 16h** | Closest to the cutoff, still clears it. Single patch
([#1493](petyosi/react-virtuoso#1493)) fixing a
one-frame blank when an older page is prepended — directly in the
log-viewer path. Log viewer captured in the visual test. |
| #1706 | `eslint` | 10.8.0 → 10.10.0 | 2026-09-04T14:34:21Z | 4d 16h |
Dev-only, but the **largest graph churn** — see below. |

## Graph movement beyond the named targets

Every item below was independently age-checked against the same cutoff
and attributed to a named cause.

**Go — lockstep and upstream requirement bumps:**

| Module | Old → New | Published | Why |
|---|---|---|---|
| `modernc.org/libc` | 1.74.4 → 1.75.6 | 2026-08-26 | **Required
lockstep.** `modernc.org/sqlite` v1.58.0 explicitly requires downstreams
to pin the exact `libc` it pins ([issue
#177](https://gitlab.com/cznic/sqlite/-/issues/177)). Verified: sqlite's
`go.mod` pins `libc v1.75.6` / `memory v1.12.1`, and ours resolves to
exactly those. |
| `modernc.org/memory` | 1.11.0 → 1.12.1 | 2026-08-19 | Same lockstep. |
| `modernc.org/cc/v4` | 4.29.1 → 4.29.2 | 2026-07-28 | Build-time
transpiler deps of libc. |
| `modernc.org/ccgo/v4` | 4.34.6 → 4.35.0 | 2026-08-06 | Same. |
| `modernc.org/gc/v3` | 3.1.4 → 3.1.5 | 2026-06-24 | Same. |
| `github.com/stretchr/testify` | 1.11.1 → 1.12.1 | 2026-08-17 | Pulled
by `prometheus/common`. Test-only. Fixes plus a swap to
`go.yaml.in/yaml/v3`. |
| `go.yaml.in/yaml/v3` | 3.0.4 → 3.0.5 | 2026-07-26 | Pulled by
`prometheus/common`/testify. Reviewed the full commit range:
test-infrastructure cleanup, a gofmt revert, and retraction of
uninstallable v3 tags. **No parser behavior change.** |

**npm — all from eslint's `file-entry-cache` v11 update
([eslint#20801](eslint/eslint#20801

`file-entry-cache` 8.0.0 → 11.1.5, `flat-cache` 4.0.1 → 6.1.23, `keyv`
4.5.4 → 5.6.0 (three major jumps), `flatted` 3.4.2 → 3.4.4,
`@eslint/plugin-kit` 0.7.2 → 0.7.3; **added** `@cacheable/memory`,
`@cacheable/utils`, `@keyv/bigmap`, `@keyv/serialize`, `cacheable`,
`hashery`, `hookified` (1.15.1 + 2.2.0), `qified`; **removed**
`json-buffer`. Oldest addition is `@keyv/serialize` (2025-09-10), newest
`nanoid` 3.3.18 (2026-08-07, via postcss) — all far past the cutoff.
This is eslint's `--cache` layer only; it is not in any runtime path and
not in the shipped bundle.

This churn is the one thing in the batch I'd flag for a second look. It
is well-soaked, fully attributed to a single deliberate upstream change,
and dev-only — but it is three major transitive jumps to buy two eslint
patch releases. Happy to drop #1706 and keep the other nine if you'd
rather not carry it this week.

## Verification

Run against the batch as a whole, from a clean worktree on
`origin/main`:

- `make tsc` — **clean.** Load-bearing here: react-query 5.102.0 changed
several public types (`NoInfer` revert
[#11245](TanStack/query#11245), `queryOptions`
return types [#11224](TanStack/query#11224),
`UseInfiniteQueryOptions` TData default
[#11147](TanStack/query#11147)) and postcss
8.5.28 fixes a types regression.
- `make test` — **all packages pass, 0 failures.** Consumer packages
re-run uncached (`-count=1`): `internal/auth` (go-oidc),
`internal/timeline` (sqlite, incl. the corruption-marker tests),
`internal/upgrade` (expfmt), `internal/ai` (sqlite), `internal/mcp` —
all ok.
- `make build` — **succeeds** end to end (frontend → embed → binary).
- `go mod verify` — **all modules verified.**
- Cross-compile — `windows/amd64`, `windows/arm64`, `linux/amd64`,
`linux/arm64`, `darwin/arm64` all build (targeted at the `x/sys` bump).
- **visual-test: ran** · 3 shots ·
`/Users/nadaverell/workspace/sky/ws5/radar/.playwright-mcp/visual-test/depsbatch-20260909/`
— run against `kind-radar-gpu-ecosystem-demo`. Not skipped, because
postcss (CSS output), react-virtuoso (list rendering), react-router
(routing) and react-query (data layer) can all materially change what
renders. Overview, Resources/Pods table, and the pod **log viewer** (the
virtuoso prepend path) all render correctly; console clean apart from an
environmental `404 /api/metrics/...` (demo cluster has no
metrics-server).

Two pre-existing conditions confirmed identical on `origin/main` and
unrelated to this batch: 4 `go vet` "copies lock" warnings in
`internal/server` test files (CI doesn't run vet), and a `darwin/amd64`
build failure in `cmd/desktop` (cgo/build-tag issue with
`startNativeMouseMonitor`).

`eslint` is the one dependency whose targeted test I did **not** run
locally — `npm run lint` is flagged broken in `.claude/commands/qa.md`.
CI does run it (`.github/workflows/ci.yml` "Lint" step), so the eslint
bump is verified there instead, as a **before/after comparison**:

| eslint | Commit | Result |
|---|---|---|
| 10.8.0 | `750cea6c` (this PR's base,
[run](https://github.com/skyhook-io/radar/actions/runs/34225225175)) | ✖
353 problems (**0 errors**, 353 warnings) |
| 10.10.0 | this PR
([run](https://github.com/skyhook-io/radar/actions/runs/34322370732/job/102371700492))
| ✖ 353 problems (**0 errors**, 353 warnings) |

Identical. 10.9.0/10.10.0 fix several rule false negatives (`new-cap`,
`no-extra-bind`, `no-loss-of-precision`, `no-unexpected-multiline`), so
new findings were possible — none materialised on Radar's source. No
source change was needed to land the bump.

## CI

All checks green on the batch: Backend, Frontend, pkg (shared library),
k8s-ui (shared UI), Helm chart, CodeQL (go + javascript-typescript),
Cursor Bugbot (no findings).

## Scope

Dependency manifests only — `go.mod`, `go.sum`, `package-lock.json`,
`web/package.json`, `packages/k8s-ui/package.json`. No application
source, generated frontend bundle, workflow, tag, or publishing config
changed. `git diff --check` clean; the root `dompurify` override is
untouched. No PR was stale/already-landed, and nothing required a Radar
source change or compatibility shim.

Supersedes #1697, #1698, #1699, #1700, #1701, #1702, #1703, #1704,
#1705, #1706.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01XArawWJfTECJQAjnWG9L1X

Co-authored-by: Claude Opus 5 (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