perf(iceberg): bound rewrite-manifests memory + calibrate manifest size estimate - #185

Picks up risingwavelabs/iceberg-rust#185 (merged as 0b65d716), the
direct child of the current pin 08a18f4. #185 bounds rewrite-manifests
memory (incremental flush of sealed manifest writers) and calibrates the
manifest size estimate. No API changes; whole-workspace `cargo check` is
clean.

`cargo fmt --all -- --check` (pinned nightly-2025-10-27) fails on
writer.rs:223, a pre-existing formatting violation introduced by #185
that was never fmt-checked (the base branch only runs scheduled
workflows on push, not the pull_request `check` job). Because fmt runs
workspace-wide, it blocks any PR against this base — unrelated to the
string-bound fix in this PR, but folded in here to get CI green.

* fix(transaction): account for added delete files in snapshot summary (risingwavelabs#166)

SnapshotProducer::summary() fed only added_data_files into the
SnapshotSummaryCollector. The added_delete_files vec is written to
delete manifests by manifest_file(), but never reached the collector,
so added-delete-files / added-position-deletes / added-equality-deletes
were never emitted and update_totals rolled total-delete-files /
total-position-deletes / total-equality-deletes forward as
previous(0) + added(0) on every commit — summaries reported zero delete
files on tables whose manifest lists carried hundreds of live ones.
Engines that consult the summary to pick a read path (or to reject
unsupported delete types up front) then mis-plan against the actual
manifests.

Add the missing added_delete_files loop in summary(), mirroring the
added_data_files one; the collector already branches on
data_file.content to update the matching position/equality delete
counters.

Add a regression test that builds a producer with one data file plus
position- and equality-delete files against a parent snapshot with
non-zero delete totals, asserting the added-* delete counters are
emitted and the total-* counters roll forward instead of staying at
zero.

* fix(writer): sort position-delete entries globally by `(file_path, pos)` (risingwavelabs#167)

* fix(writer): sort position-delete entries globally by (file_path, pos)

DeltaWriter previously called write_position_deletes() once per
insert/delete pass, producing multiple independently sorted runs
concatenated into a single parquet file. Readers that rely on the
Iceberg v2 sort guarantee would fail to apply deletions correctly.

Buffer all position-delete inputs in a BTreeMap<Arc<str>, Vec<i64>>
keyed by file path. At close(), drain the buffer and emit it sorted
by (file_path, pos) in fixed-size chunks so the underlying
RollingFileWriter retains its file-splitting behavior.

Add two regression tests that confirm per-file and cross-write global
ordering.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

* style(writer): apply rustfmt formatting to delta_writer

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

---------

Co-authored-by: Claude Sonnet 4.6 <[email protected]>

* feat(writer): auto-set referenced_data_file on PositionDeleteFileWriter (risingwavelabs#169)

* feat(writer): auto-set referenced_data_file on PositionDeleteFileWriter close

When a position delete writer closes with exactly one output file and all
written entries reference a single distinct data file path, set
referenced_data_file on the resulting DataFile. This allows readers to
scope delete file application without scanning the file_path column,
matching Iceberg Java behavior and eliminating unnecessary I/O.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

* fix(writer): set referenced_data_file on all rolled position delete files

The builders.len() == 1 guard prevented referenced_data_file from being
set when the rolling writer produced multiple output files due to size
rollover. Since rolling is purely size-driven, distinct_paths.len() == 1
already guarantees that every rolled output file contains entries for
the same single data file. Removing the builders.len() check ensures the
field is set on all output files, so readers can skip them for
non-matching data files even in high-volume single-file delete scenarios.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

* refactor(writer): clean up test comment in position_delete_file_writer

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

---------

Co-authored-by: Claude Sonnet 4.6 <[email protected]>

* fix: stream manifest lists during snapshot expiration to bound memory (risingwavelabs#170)

RemoveSnapshotAction::commit, when clear_expired_meta_data is enabled,
computes the set of reachable partition-spec ids by calling
load_manifest_lists(retained_snapshots).try_collect(), which holds every
retained snapshot's manifest list in memory at once. It only needs the small
set of reachable spec ids.

On tables whose retained manifest lists are large (e.g. high-churn tables
without manifest compaction), this is O(N) in memory and can OOM the process
during commit.

Add a streaming helper `for_each_manifest_list` that loads manifest lists with
bounded concurrency (buffer_unordered) and invokes a closure on each, dropping
the list immediately afterwards. RemoveSnapshotAction now folds the reachable
partition-spec ids through it instead of collecting a Vec. The result is
identical; peak memory drops from O(all retained manifest lists) to
O(concurrency lists in flight). reachable_schemas already comes from
Snapshot::schema_id() and needs no manifest load.

* fix: stream manifest loading in snapshot-expiration file cleanup (risingwavelabs#171)

* fix: stream manifest lists during snapshot expiration to bound memory

RemoveSnapshotAction::commit, when clear_expired_meta_data is enabled,
computes the set of reachable partition-spec ids by calling
load_manifest_lists(retained_snapshots).try_collect(), which holds every
retained snapshot's manifest list in memory at once. It only needs the small
set of reachable spec ids.

On tables whose retained manifest lists are large (e.g. high-churn tables
without manifest compaction), this is O(N) in memory and can OOM the process
during commit.

Add a streaming helper `for_each_manifest_list` that loads manifest lists with
bounded concurrency (buffer_unordered) and invokes a closure on each, dropping
the list immediately afterwards. RemoveSnapshotAction now folds the reachable
partition-spec ids through it instead of collecting a Vec. The result is
identical; peak memory drops from O(all retained manifest lists) to
O(concurrency lists in flight). reachable_schemas already comes from
Snapshot::schema_id() and needs no manifest load.

* fix: stream manifest loading in snapshot-expiration file cleanup

ReachableFileCleanupStrategy (used after a remove_snapshots commit when
clear_expired_files is enabled) loaded entire collections up front:
- clean_files / prune_referenced_manifests collected
  Vec<(SnapshotRef, ManifestList)> via load_manifest_lists, then folded the
  entries into HashSets.
- find_files_to_delete collected Vec<(ManifestFile, Manifest)> via
  load_manifests, then folded data-file paths into a HashSet.

In each case the fully-loaded Vec was held in memory simultaneously with the
derived set, roughly doubling peak memory on tables with many/large
manifests.

Add a streaming `for_each_manifest` helper (analogous to
`for_each_manifest_list`) and convert the three cleanup sites to fold entries
into their sets while dropping each loaded list/manifest immediately. Peak
memory for the load drops from O(all loaded lists/manifests + derived set) to
O(concurrency in flight + derived set). Behavior is unchanged.

Note: this bounds the *loading*; the derived sets (manifest files, data-file
paths) are still O(N) by nature of the diff-based cleanup algorithm. Fully
bounding cleanup needs a batched/partitioned deletion strategy (separate
follow-up).

* fix: stream manifest loading in rewrite-manifests and append/overwrite validation (risingwavelabs#172)

* fix: stream manifest loading in rewrite-manifests and append/overwrite validation

Two more spots loaded an entire manifest collection up front and held it for
the duration of a fold:

- rewrite_manifests routes entries from `manifests_to_rewrite` into clustered
  manifest writers, but first collected every manifest via load_manifests.
- the append/overwrite validation path scans the current snapshot's manifests
  to detect duplicate adds and validate deletes; it also loaded all of them via
  load_manifests before scanning, even though it early-exits once both checks
  are satisfied.

Add a fallible streaming helper `try_for_each_manifest` (analogous to
`for_each_manifest`) and use it in rewrite_manifests to route entries while
dropping each manifest immediately. Convert the validation scan to a streamed
`buffer_unordered` loop that drops each manifest after scanning and stops
pulling manifests entirely once the early-exit condition is met.

Behavior is unchanged (both paths already processed manifests in
buffer_unordered completion order). Peak memory drops from O(all loaded
manifests) to O(concurrency in flight), and the validation path additionally
avoids loading manifests past the early-exit.

* docs: fix stale comment in append/overwrite validation scan

The leading comment said the block loads all manifests concurrently, but
it now streams manifests one at a time with early-exit. Update it to
describe the scan's purpose, removing the contradiction with the streaming
note below.

* Add rewrite manifests target size (risingwavelabs#174)

* fix: stream manifest loading in rewrite-manifests and append/overwrite validation

Two more spots loaded an entire manifest collection up front and held it for
the duration of a fold:

- rewrite_manifests routes entries from `manifests_to_rewrite` into clustered
  manifest writers, but first collected every manifest via load_manifests.
- the append/overwrite validation path scans the current snapshot's manifests
  to detect duplicate adds and validate deletes; it also loaded all of them via
  load_manifests before scanning, even though it early-exits once both checks
  are satisfied.

Add a fallible streaming helper `try_for_each_manifest` (analogous to
`for_each_manifest`) and use it in rewrite_manifests to route entries while
dropping each manifest immediately. Convert the validation scan to a streamed
`buffer_unordered` loop that drops each manifest after scanning and stops
pulling manifests entirely once the early-exit condition is met.

Behavior is unchanged (both paths already processed manifests in
buffer_unordered completion order). Peak memory drops from O(all loaded
manifests) to O(concurrency in flight), and the validation path additionally
avoids loading manifests past the early-exit.

* docs: fix stale comment in append/overwrite validation scan

The leading comment said the block loads all manifests concurrently, but
it now streams manifests one at a time with early-exit. Update it to
describe the scan's purpose, removing the contradiction with the streaming
note below.

* feat(rewrite_manifests): bound output manifest size to target (8 MiB)

The cluster_by path of RewriteManifestsAction routed every alive entry for a
partition cluster key into a single ManifestWriter, producing one unbounded
manifest per key. On high-ingest tables this yields huge manifests and an
oversized manifest list.

Add size-based rollover honoring commit.manifest.target-size-bytes (default
8 MiB), matching Spark and the append/overwrite/rewrite_files paths:
- ManifestWriter tracks a running estimated serialized size and exposes
  estimated_manifest_size(); estimate captures file path + per-column stat
  maps + fixed overhead.
- When an open writer reaches the target it is sealed (deferred finalize,
  since the streaming routing closure is sync) and the next entry for that
  key opens a fresh writer.

File count is preserved (created==replaced still holds); manifest ordering is
not required for correctness. Adds a unit test for the size signal.

* test(rewrite_manifests): cover target-size manifest rollover

Add integration test requested in PR review: set
commit.manifest.target-size-bytes=1 so the cluster_by path rolls each
alive entry into its own output manifest, verifying that a hot cluster
key no longer produces a single unbounded manifest.

Refactors the shared setup helper to accept table properties on
creation.

* chore: update code owners (add @netgusto & @agaddis02, remove @jonathanc-n) (risingwavelabs#176)

* chore: add @netgusto to code owners

* chore: remove @jonathanc-n; sync owner list

* chore: add @agaddis02 to code owners

* fix(rewrite_manifests): honor target size from snapshot properties (risingwavelabs#175)

The rewrite-manifests target size (`commit.manifest.target-size-bytes`)
was only read from the persistent *table* properties. Callers that set
it per-commit via `set_snapshot_properties` (the transient, non-persisted
mechanism) had no effect on manifest sizing — the value was recorded in
the snapshot summary but ignored when writing manifests.

Resolve the target size in order: staged snapshot property (this action's
`set_snapshot_properties`) first, then the table property, then the
Iceberg default (8 MiB). This lets a caller size the manifests for a
single rewrite without mutating persistent table configuration, while
preserving the existing table-property and default behavior.

Add an integration test proving the snapshot property overrides a
conflicting table property and does not mutate it.

* feat: Variant Support (risingwavelabs#145)

* cherry-pick feat: Variant Support

* Preserve Arrow variant extension metadata

* Fix Arrow variant projection and schema conversion

* Deduplicate variant integration tests by extracting shared helpers

Extract `load_variant_table()` and `assert_variant_column()` to remove
duplicated catalog setup and variant column assertions across tests.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix(avro): derive unique variant record names from enclosing field id

Avro record names must be unique within a schema, so a fixed "variant"
record name breaks schemas with more than one variant column. Use r{field_id}
derived from the enclosing field id, mirroring Java's TypeToSchema and the
current apache#2188 revision.

Co-Authored-By: Claude Fable 5 <[email protected]>

* fix(reader): reject shredded variant columns in projection

A `typed_value` sub-field means the variant payload is shredded, which we
cannot reconstruct yet. Projecting only metadata/value would silently drop
data, so fail loudly instead (ported from apache#2188).

Co-Authored-By: Claude Fable 5 <[email protected]>

* fix(reader): derive variant extension metadata from Parquet VARIANT logical type

Files written by Spark/iceberg-java annotate variant groups with the Parquet
VARIANT logical type but embed no Arrow schema, so the Arrow schema used for
projection is derived from the Parquet schema at read time. Without parquet's
variant_experimental feature the derivation produced a plain struct with no
extension metadata, and arrow_schema_to_schema recursed into the id-less
variant sub-fields, failing with "Field id not found in metadata".

Enable variant_experimental so the derived schema carries
arrow.parquet.variant, which the schema converter and projection rely on.

Co-Authored-By: Claude Fable 5 <[email protected]>

* fix: auto-select format version for DataFusion-created tables

Port the check_format_compatibility scope changes from the current
apache#2188 revision, previously skipped to keep divergence
minimal. The cherry-picked check validates table metadata at creation, which
broke DataFusion CREATE TABLE with nanosecond timestamps (DataFusion's default
TIMESTAMP) on the default v2 format: df_scan_all_types.toml and
df_timestamp_predicate_pushdown.toml failed with "Invalid schema for vv2".
These failures were masked in CI by fail-fast on the (now fixed) read_variant
integration tests.

- DataFusion CREATE TABLE picks the format version from
  Schema::calc_min_compatible_format() (at least v2, v3 when the schema
  requires it, e.g. timestamp_ns / variant).
- check_format_compatibility now also validates initial_default floors,
  reports problems sorted by field id, and fixes the doubled "vv2" in the
  message (FormatVersion's Display already includes the "v").
- Table metadata validation checks only the current schema, matching the
  upstream revision.

Co-Authored-By: Claude Fable 5 <[email protected]>

* chore: adopt remaining upstream apache#2188 deltas to minimize divergence

Port everything still differing from the current apache#2188
revision, except the fork's deliberate fixes (variant extension metadata,
projection lock-step traversal, variant_experimental derivation):

- Glue/HMS catalogs map variant column types ("variant" / "unknown",
  matching iceberg-java's HiveSchemaUtil, apache/iceberg#15964) instead of
  rejecting with FeatureUnsupported.
- Port upstream's variant tests for partition/sort source rejection, arrow
  value conversion, id reassignment, schema indexing, column pruning,
  parquet path indexing, and type_to_arrow_type.
- Align index.rs/prune_columns.rs import style with upstream (files now
  byte-identical).

Not ported: transaction/update_schema.rs hunks (patch code the fork's older
base does not have) and public-api.txt (mechanism absent from this base).

Co-Authored-By: Claude Fable 5 <[email protected]>

---------

Co-authored-by: Christian Thiel <[email protected]>
Co-authored-by: Claude Opus 4.6 <[email protected]>

* fix: rewrite/overwrite transactions mishandle DELETE manifests (risingwavelabs#179)

* fix: overwrite txn corrupts DELETE manifest

Signed-off-by: Mingzhuo Yin <[email protected]>

* fix rewrite txn

* fix: preserve delete-manifest content type and entry status on rewrite

`existing_manifest` rewrites a manifest in place when only some of its
entries are removed. Two defects in that path, both present in the
verbatim-duplicated `overwrite_files` / `rewrite_files` twins:

1. The writer was opened with a hardcoded `ManifestContentType::Data`.
   For a delete manifest this makes `check_data_file` reject the first
   surviving entry, so any overwrite/rewrite that removes a subset of the
   delete files tracked by a delete manifest fails the whole commit with
   `DataInvalid`. This is the common case for compaction on a table with
   row-level deletes. Pass the source manifest's own content through.

2. Surviving entries were carried forward with `add_entry`, which
   restamps them as `Added` under the new snapshot and clears their file
   sequence number. This inflates `added_files_count` / `added_rows_count`
   and makes an unchanged file look newly added to incremental readers.
   Use `add_existing_entry`, skipping tombstones first, matching what
   `rewrite_manifests` and `manifest_filter` already do.

Also fixes an operator-precedence bug in `delete_entries`
(`overwrite_files` only): `A || B && C` parses as `A || (B && C)`, so
every `PositionDeletes` entry was marked `Deleted` regardless of whether
its path was in `removed_delete_file_paths`.

`add_existing_entry` now has a real caller, so drop its stale
`#[allow(dead_code)]`.

Adds four regression tests over a shared fixture, each verified to fail
against the corresponding pre-fix behaviour.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

---------

Signed-off-by: Mingzhuo Yin <[email protected]>
Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>

* refactor: unify overwrite & rewrite txn (risingwavelabs#182)

* chore: add silver-ymz to code owners (risingwavelabs#183)

* Improve RewriteManifestsAction to reuse rewritten manifests on conflict retry (risingwavelabs#181)

* perf(iceberg): stream manifest loading in orphan file removal (risingwavelabs#177)

Orphan file removal loaded all manifest lists and manifests into memory
simultaneously before scanning, holding O(all snapshots × manifests) in
peak memory.

collect_snapshot_files() used load_manifest_lists and load_manifests
which collect every loaded item into a Vec before the caller can fold them.
On tables with many snapshots or large manifests, this could OOM the
process during orphan cleanup.

Replaced load_manifest_lists/load_manifests with the streaming helpers
for_each_manifest_list/for_each_manifest (introduced upstream in PRs
risingwavelabs#170-risingwavelabs#172). Manifest list paths are extracted from snapshots upfront,
then each manifest list and manifest is streamed with bounded concurrency
and dropped immediately after its entries are folded into the reachable
set. Removed load_manifest_lists from utils.rs as it now has zero callers.

* feat: add some trival traits for DeleteVector (risingwavelabs#184)

Signed-off-by: Mingzhuo Yin <[email protected]>

* perf(iceberg): bound rewrite-manifests memory + calibrate manifest size estimate (risingwavelabs#185)

* perf(iceberg): bound rewrite-manifests memory + calibrate size estimate

Two related improvements to the size-bounded `RewriteManifestsAction`
cluster path added in risingwavelabs#174.

1. Incremental flush (memory) — IGLOO-283. Seal and write each full
   manifest writer inside the streaming loop instead of parking every
   sealed writer in a `full_writers` buffer until the end. Previously a
   large single-key (e.g. unpartitioned) rewrite buffered every output
   entry in memory at once — O(total entries) — and could OOM the
   process (observed: a ~10k-entry, 1000-column table OOMKilled a 2 GiB
   worker). Peak memory is now bounded by the in-flight input manifests
   plus the still-open (below-target) writers (<=1 per active cluster
   key). The load stream keeps its bounded concurrency; entry routing
   stays sequential because writers are stateful.

2. Calibrate `estimate_manifest_entry_size` — IGLOO-284. The flat per-KV
   constants overcounted the real (uncompressed) Avro footprint by ~1.7x,
   so writers sealed at ~0.6x the target and produced manifests well
   under `commit.manifest.target-size-bytes`. Iceberg encodes the stat
   maps as Avro arrays of {key:int, value:...} records, so per-KV costs
   are much smaller; the bound maps now also count the real serialized
   value bytes (via a non-allocating `datum_serialized_len`) so
   wide/untruncated bounds are no longer undercounted. Measured within
   ~15% of the on-disk length on a 60-column manifest (vs ~1.85x before).

Also removes the now-unused `try_for_each_manifest` helper: the cluster
path was its only caller, and the loop is inlined so `write_manifest_file`
can be awaited at seal time.

Tests:
- writer: `test_estimated_size_matches_on_disk_manifest` asserts the
  estimate is within +/-30% of a real on-disk manifest length (fails
  under the old constants).
- rewrite_manifests: `test_rewrite_manifests_rolls_over_and_preserves_count`
  asserts a hot key splits into multiple manifests with no entry lost.

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <[email protected]>

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <[email protected]>

---------

Co-authored-by: Copilot Autofix powered by AI <[email protected]>

---------

Signed-off-by: Mingzhuo Yin <[email protected]>
Co-authored-by: Tran Ngoc Tuan <[email protected]>
Co-authored-by: Claude Sonnet 4.6 <[email protected]>
Co-authored-by: vovacf201 <[email protected]>
Co-authored-by: Yuhao Su <[email protected]>
Co-authored-by: Christian Thiel <[email protected]>
Co-authored-by: Mingzhuo Yin <[email protected]>
Co-authored-by: Dylan <[email protected]>
Co-authored-by: Alex Graham <[email protected]>
Co-authored-by: Copilot Autofix powered by AI <[email protected]>
Co-authored-by: Cursor <[email protected]>

's decimal arm was left unformatted by risingwavelabs#185, which makes
`make check-fmt` fail on `dev_rebase_main_20260303`. CI has been red on the
branch since (`check` on both runners, plus `check-rust` in the Python bindings
workflow, which reaches the same file through its path dependency), so every PR
opened against it inherits the failure.

Formatting only -- `cargo fmt --all` with the pinned nightly-2025-10-27
toolchain, no functional change.

* fix(spec): decode truncated string metric bounds leniently

String lower/upper bounds in manifest entries are truncated to a fixed
byte length by the metrics truncation config (default truncate(16)). A
writer that truncates on a raw byte boundary rather than a UTF-8
character boundary can split a multi-byte character, leaving an
incomplete UTF-8 sequence in the stored bytes.

Datum::try_from_bytes decoded these with a strict std::str::from_utf8,
which errored ('handling invalid utf-8 characters' /
'incomplete utf-8 byte sequence from index N') and made the entire
manifest unreadable — breaking compaction for the affected table.

Decode leniently with String::from_utf8_lossy instead. The value is only
ever used as a scan-pruning bound, so replacing an invalid truncated
tail with U+FFFD is acceptable and keeps the manifest loadable. This
matches the from_utf8_lossy handling already used for manifest metadata
fields (schema-id, partition-spec-id, content).

* style: rustfmt writer.rs (pre-existing drift from #185)

`cargo fmt --all -- --check` (pinned nightly-2025-10-27) fails on
writer.rs:223, a pre-existing formatting violation introduced by #185
that was never fmt-checked (the base branch only runs scheduled
workflows on push, not the pull_request `check` job). Because fmt runs
workspace-wide, it blocks any PR against this base — unrelated to the
string-bound fix in this PR, but folded in here to get CI green.

…itten (#188)

* fix(iceberg): drop dangling deletion vectors when data files are rewritten

`ManifestFilterManager::remove_dangling_deletes_for` recorded the rewritten data
file paths but nothing ever consumed them: `removed_data_file_path` was
write-only, its single other use being an emptiness test in
`can_contain_deleted_files`. The check that was supposed to use it was left as a
TODO in `manifest_has_deleted_files`:

    // TODO: Add dangling delete vector check: (is_delete && self.is_dangling_dv(file))

As a result, a rewrite (e.g. compaction) that replaces a data file leaves that
file's deletion vector live in the delete manifests forever. The DV applies to
nothing — readers bind a DV to a data file by `referenced-data-file` equality,
and that data file is gone — but it stays *reachable*, so:

- delete manifests grow monotonically with every rewrite,
- the Puffin file is pinned alive, so snapshot expiration cannot reclaim it,
- scan planning keeps paying for delete entries that can never apply.

The only existing mitigation is the coarse sequence-number rule in
`drop_delete_files_older_than`, which is opportunistic: it cannot fire while any
older data manifest survives.

This implements the missing check as `is_dangling_delete` and wires it into both
places that compute `marked_for_delete` (the scan in `manifest_has_deleted_files`
and the rewrite in `filter_manifest_with_deleted_files`), so a delete manifest is
only rewritten when it actually contains a dangling entry.

A delete is treated as dangling only when it can be *proven* to apply to nothing:
it must be a non-data file whose `referenced-data-file` is in the set of data
files being removed by this commit. That covers v3 deletion vectors and v2
position delete files that set `referenced-data-file`. Equality deletes, and
position deletes that span several data files, have no single referenced data
file and may still apply to surviving data files, so they are never reported as
dangling and continue to be retired by the sequence-number rule.

Tests: `test_is_dangling_delete` covers the predicate including the
must-not-drop cases (DV for a surviving data file, a data file itself, and a
delete without `referenced-data-file`); `test_filter_manifests_drops_dangling_
deletion_vector` asserts end-to-end that the DV of a rewritten data file is
marked `Deleted` in the rewritten manifest while the DV of a surviving data file
is kept as `Existing`; and
`test_filter_manifests_keeps_deletion_vectors_when_nothing_removed` asserts the
no-op path does not rewrite the manifest.

* style: fix rustfmt violation in manifest writer

's decimal arm was left unformatted by #185, which makes
`make check-fmt` fail on `dev_rebase_main_20260303`. CI has been red on the
branch since (`check` on both runners, plus `check-rust` in the Python bindings
workflow, which reaches the same file through its path dependency), so every PR
opened against it inherits the failure.

Formatting only -- `cargo fmt --all` with the pinned nightly-2025-10-27
toolchain, no functional change.

* ci: pin ruff for the Python bindings lint job

`uv tool install ruff` resolves the latest release, so a new ruff version changes
what CI enforces with no change to this repo. ruff 0.16.0 expanded the default
rule selection and began reporting 16 findings in `bindings/python`
(DTZ001 x5, UP006 x3, UP035 x3, I001 x3, PLR0402, PIE804), so `check-python`
turned red on every PR -- including PRs that touch no Python at all.

Pin to 0.15.22, the last version this repo was green on, verified from the CI
logs: the last successful `bindings_python_ci` run installed ruff 0.15.22 and
reported no findings, while the failing runs install 0.16.0. This matches how the
same job already pins `uv` itself to 0.9.3.

Bumping ruff should then be a deliberate change, made together with whatever
fixes the newer version asks for, rather than arriving unannounced and blocking
unrelated work. `[tool.ruff.lint]` in `pyproject.toml` sets only `ignore`, with no
`select`, so this directory is fully exposed to default-rule-set changes; adding
an explicit `select` would be the more durable follow-up.

* fix(iceberg): never promote a dangling deletion vector to a path-level delete

Review catch by @chenzl25, and a real correctness bug in the first version of this
change.

Several deletion vectors can live in ONE Puffin file, distinguished by
(`file_path`, `content_offset`, `content_size_in_bytes`), so danglingness is a
property of a single *blob*, not of the file. The previous commit routed a
dangling blob through the same branch as an explicit delete, which inserts the
file into `files_to_delete` -- a map keyed by **path**. That path then matched
every other entry with the same Puffin path, in this manifest and in every later
one (the manager is reused across `filter_manifests`), dropping the *live*
deletion vectors of data files that were never rewritten and silently resurrecting
their deleted rows. It could also queue a still-referenced Puffin for physical
deletion via `files_to_be_deleted`.

Fix: keep the three removal reasons separate -- `explicitly_deleted`,
`superseded_by_sequence_number`, `dangling_delete` -- and only promote to the
path-level sets for the first two, which do condemn the whole file. A dangling
blob now has exactly one effect: that single entry is written as deleted.

The sequence-number rule is left as-is: entries sharing a Puffin were written by
the same commit and so share a sequence number, meaning they are retired together
and consistently.

Regression test `test_shared_puffin_keeps_live_deletion_vectors` builds one Puffin
holding a dangling blob plus a live blob, and a second manifest holding another
live blob of the same Puffin. It asserts, identifying entries by blob offset since
they share a path, that: the dangling blob is `Deleted`; the live blob is
`Existing`; the second manifest is not rewritten at all (no cross-manifest
contamination); and the Puffin is never added to `files_to_be_deleted`. Verified
the test is not vacuous -- restoring the unconditional promotion makes it fail on
the "must survive" assertion.