Skip to content

fix: module compression incorrectly reused first file's root pointer#5

Merged
digama0 merged 1 commit into
digama0:masterfrom
kim-em:fix_module_bug
Oct 23, 2025
Merged

fix: module compression incorrectly reused first file's root pointer#5
digama0 merged 1 commit into
digama0:masterfrom
kim-em:fix_module_bug

Conversation

@kim-em

@kim-em kim-em commented Oct 22, 2025

Copy link
Copy Markdown
Contributor

Summary

Fixes a bug in the module compression feature where .olean.server and .olean.private files were being truncated to 96 bytes (just the header) instead of being fully compressed.

Note: This diagnosis and fix were performed by Claude Code (@anthropics). While integration tests pass, this needs careful review by @digama0 to ensure correctness, as I (@kim-em) cannot vouch for the implementation details.

The Bug

Two related issues in src/lgz.rs compress() function:

  1. Line 261: Used root instead of header2.root when storing each file's metadata

    • Caused all Data entries to reference the same root object from the first file
  2. Line 279: Used root instead of d.root during reference counting

    • Only marked the first file's root as referenced

Symptom

When compressing three module files (.olean, .olean.server, .olean.private):

  • ✅ First file (.olean): Compressed correctly → ~48KB
  • ❌ Second file (.server): Decompressed to 96 bytes instead of 2.3KB
  • ❌ Third file (.private): Decompressed to 96 bytes instead of 46KB

The second and third files were truncated because the compressor's backref cache thought it had already written their root objects (since all three files pointed to the first file's root).

The Fix

Two one-line changes in src/lgz.rs:

  1. Line 261: rootheader2.root
  2. Line 279: rootd.root

Each file now correctly stores and compresses its own root pointer, producing three complete, distinct object graphs.

Testing

#4 added integration tests in tests/module_compression.rs with test data from Plausible.Attr module.

Before fix:

✓ test_single_olean_roundtrip: PASSED
✗ test_module_roundtrip: FAILED
  - Attr.olean.server: 2344 bytes → 96 bytes (truncated!)
  - Attr.olean.private: 47320 bytes → 96 bytes (truncated!)

After fix:

✓ test_single_olean_roundtrip: PASSED  
✓ test_module_roundtrip: PASSED
  - Attr.olean:         48976 bytes ✓
  - Attr.olean.server:  2344 bytes ✓
  - Attr.olean.private: 47320 bytes ✓

cc @digama0 for review

@kim-em

kim-em commented Oct 22, 2025

Copy link
Copy Markdown
Contributor Author

(If anyone is interested in what the Claude conversation looked like, it's here.)

The module compression feature (added in commits 067cd65 and ff253cb)
had a bug where all three module files (.olean, .olean.server,
.olean.private) were compressed using the first file's root pointer.

Bug diagnosis and fix by Claude (@anthropics).

## The Bug

Two related issues in src/lgz.rs compress():

1. Line 261: Used `root` instead of `header2.root` when storing each
   file's metadata. This caused all Data entries to reference the same
   root object from the first file.

2. Line 279: Used `root` instead of `d.root` during reference counting,
   only marking the first file's root as referenced.

## The Symptom

When compressing three module files together:
- First file (.olean): Compressed correctly → ~48KB
- Second file (.server): Decompressed to 96 bytes instead of 2.3KB
- Third file (.private): Decompressed to 96 bytes instead of 46KB

The second and third files were truncated to just the olean header
because the compressor thought it had already written their root objects
(since they all pointed to the first file's root).

## The Fix

Two one-line changes:
1. Line 261: `root` → `header2.root`
2. Line 279: `root` → `d.root`

Each file now correctly stores and compresses its own root pointer,
producing three complete, distinct object graphs.

## Testing

Added integration tests in tests/module_compression.rs with test data
from Plausible.Attr module. Tests verify byte-identical roundtrip for
both single-file and multi-file (module) compression.

Before fix: test_module_roundtrip FAILED (files truncated to 96 bytes)
After fix: test_module_roundtrip PASSED (all files match perfectly)

Note: This diagnosis and fix were performed by Claude Code. While tests
pass, this needs careful review by @digama0 to ensure correctness.

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

Co-Authored-By: Claude <noreply@anthropic.com>
@digama0
digama0 merged commit 75667e5 into digama0:master Oct 23, 2025
5 checks passed
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