# Review: DiskLoc.file_id u16 → u32 Widening

**Spec:** `armdb/docs/superpowers/specs/26-05-17-disk-loc-file-id-u32.md`
**Reviewer model:** Claude Sonnet 4.6
**Date:** 2026-05-17
**Verdict:** requires fixes before implementation

---

## Краткий итог

Спека правильно описывает проблему, цели и гарантию "без on-disk миграции". Однако инвентаризация cast-сайтов содержит два существенных расхождения с кодом: реальное число `as u16` — 19, а не 23; а список `as u32` к удалению содержит только 2 строки из shard.rs, тогда как в кодовой базе таких каста ~52 штуки в 9 разных файлах. Это значит, что план, написанный по спеке "буквально", пройдёт `cargo check` но упадёт на `cargo clippy -- -D warnings` — которое прямо указано в разделе Verification.

---

## Что хорошо

- Корень проблемы описан точно: монотонный `next_file_id: u32` + `DiskLoc.file_id: u16` = молчаливый алиасинг после 65535 ротаций.
- Обоснование ceiling-а (lifetime bytes, не active bytes) — правильное и важное.
- Утверждение "DiskLoc не сериализуется на диск" верифицировано в коде: `EntryHeader` содержит только `gsn(8) + crc32(4) + value_len(4)`, hint-файл — `GSN(8) | Key | Offset(8) | Len(4)`. Никакого `file_id` на диске нет. Обратная совместимость реальна.
- Все внутренние поля (`next_file_id`, `ActiveFile.file_id`, `ImmutableFile.file_id`, `BlockKey.file_id`, `read_block` параметр) — уже `u32`. Подтверждено по строкам: shard.rs:101, 115, 124, 405, 588. ✓
- `parse::<u32>()` везде: shard.rs:194, hint.rs:180, recovery.rs:1598, replication/log_reader.rs:352. ✓
- `config.rs:94` — cap `max_file_size > u32::MAX`. ✓
- Struct layout после изменения: `u32+u32+u32+u8+[u8;3]` = 16 байт, alignment 4. Математически верно.
- Тест-покрытие продумано хорошо: struct invariants, large file_id roundtrip, BlockKey disambiguation.
- Non-Goals чётко очерчены.

---

## Что надо исправить

### 1. Неверный счётчик `as u16` каст-сайтов: 19, а не 23

**Spec (строка 135):** `**`as u16` casts to remove** (23 sites — all inputs into `DiskLoc::new`):`

Реальный grep по `grep -rn "file_id as u16" armdb/src/`:

| Файл | Строки | Кол-во |
|---|---|---|
| `recovery.rs` | 437, 494, 547, 646, 757, 858, 1116, 1202, 1238, 1321, 1471, 1565 | 12 |
| `var_tree.rs` | 1118 | 1 |
| `var_map.rs` | 754 | 1 |
| `const_tree.rs` | 969 | 1 |
| `const_map.rs` | 728 | 1 |
| `compaction.rs` | 191, 217 | 2 |
| `shard.rs` | 792 | 1 |
| **Итого** | | **19** |

Таблица в самой спеке тоже перечисляет 19 строк — цифра 23 в тексте ошибочная. Исправить на 19.

**Влияние:** косметическое, но вызовет путаницу при ревью PR.

---

### 2. (Критическое) Инвентаризация `as u32` к удалению неполная — пройдёт `cargo check`, упадёт `cargo clippy -D warnings`

**Spec (строки 147–157):** список "as u32 casts to remove" содержит **только** shard.rs:367 и shard.rs:376. Плюс отдельно упоминается shard.rs:963 неявно как третий.

Реальный `grep -rn "file_id as u32" armdb/src/` даёт **52 строки** `DiskLoc.file_id as u32`. Полный список файлов, которые не упомянуты в разделе "Files Changed" и не упомянуты в таблице каст-сайтов, но содержат такие касты:

| Файл | Строки |
|---|---|
| `var_tree.rs` | 317, 340, 383, 413, 660, 803, 811, 819, 828, 830, 946, 1013, 1033, 1045, 1939, 2039, 2281, 2285, 2296 |
| `var_map.rs` | 279, 336, 380, 524, 564, 592, 599, 606, 614, 616, 669, 681, 1176, 1254, 1447, 1451, 1462 |
| `typed_map.rs` | 338, 402, 611, 692 |
| `typed_tree.rs` | 376, 416, 501, 553, 619 |
| `compaction.rs` | 303, 331 |
| `durability.rs` | 109, 115 |
| `shard.rs` | 367, 376, 963 |

После того как `DiskLoc.file_id` станет `u32`, все эти `disk.file_id as u32` превратятся в identity-касты `u32 as u32`. Компилятор их пропустит без ошибки, но **clippy lint `clippy::unnecessary_cast` выдаст warning на каждый**. Поскольку проверка `cargo clippy -p armdb --features var-collections -- -D warnings` включена в раздел Verification, сборка упадёт.

**Что сделать:** раздел "Call-site updates" спеки должен включать полный список всех `file_id as u32` каст-сайтов или явно сказать, что **все** `disk*.file_id as u32` (из любого `DiskLoc`-поля) должны быть очищены. Раздел "Files Changed" должен добавить `typed_map.rs`, `typed_tree.rs`, `durability.rs`.

Альтернатива (более слабая): оставить касты и убрать `-- -D warnings` из clippy-шага. Но это ухудшает качество кода.

**Примечание:** тест `var_tree.rs:1648` содержит `snap.file_id as u32` внутри `#[cfg(test)]` — это тоже станет identity-кастом, но так как clippy по умолчанию проверяет тесты, его тоже нужно убрать.

---

### 3. Ссылка `hint.rs:45-67` не доказывает то, что заявлено

**Spec:** `Hint files contain (GSN, Key, Offset, Len) — no file_id (hint.rs:45-67).`

Строки 45–67 в hint.rs — это сигнатуры функций `generate_hint_data_dyn` и `generate_hint_data_dyn_encrypted`. Они не показывают формат. Реальное доказательство — строки **103–107** (`generate_hint_data_inner`):

```rust
// Serialize: GSN(8) | Key(key_len) | Offset(8) | Len(4)
buf.extend_from_slice(&header.gsn.to_ne_bytes());
buf.extend_from_slice(&key_bytes);
buf.extend_from_slice(&value_offset.to_ne_bytes());
buf.extend_from_slice(&header.value_len.to_ne_bytes());
```

Никакого `file_id` нет — это правильный вывод. Но ссылку надо исправить на `hint.rs:103-107`.

**Влияние:** при ревью PR рецензент откроет не тот диапазон строк.

---

### 4. Третий `as u32` в shard.rs — строка 963 — не упомянут

**Spec:** "The other `as u32` casts at `shard.rs:421, 431` take a `file_id: u32` parameter — those are not DiskLoc-derived and stay."

Это верно. Но есть ещё `shard.rs:963` внутри `read_value_from_disk_locked`:

```rust
let target_file_id = loc.file_id as u32;
```

Это `DiskLoc.file_id as u32` — такой же identity-каст после fix-а. Спека не упоминает его ни в таблице `as u32 to remove`, ни в разделе shard.rs под Files Changed.

---

### 5. Тест-покрытие: нет теста для `read_value_from_disk_locked` path

Спека предлагает `read_block_with_file_id_above_u16` и `write_buffer_read_with_file_id_above_u16`. Это покрывает `Shard::read_block` (var_tree/var_map cached path). Но `read_value_from_disk_locked` (`shard.rs:961`) — отдельная функция, используемая в non-var-collections путях. Стоит добавить проверку этого пути, хотя в кодовой базе уже есть `read_value_from_disk_locked_returns_stale_for_unknown_file_id` — так что, скорее всего, достаточно убедиться, что этот тест остаётся зелёным.

Это не блокер, но стоит упомянуть в плане.

---

## Готовность

**Verdict: requires fixes before implementation.**

Обязательные правки перед написанием плана:

1. **Исправить счётчик `as u16`**: 23 → 19.
2. **Расширить таблицу `as u32 to remove`**: добавить полный список (52 строки в 9 файлах), либо написать "все `disk*.file_id as u32` касты во всех файлах". Добавить `typed_map.rs`, `typed_tree.rs`, `durability.rs` в раздел Files Changed.
3. **Исправить ссылку hint.rs**: `hint.rs:45-67` → `hint.rs:103-107`.
4. **Добавить shard.rs:963** в список `as u32 to remove`.

Правки 1, 3, 4 — косметические и быстрые. Правка 2 — содержательная и устраняет единственный риск провала clippy -D warnings на CI.
