Tier partitions in bound order and refuse to drop a non-empty partition

[vyruss]

Partitions past the hot window were tiered in name order while the
destructive branch compared bounds. With any naming whose text order differs
from calendar order (unpadded months, where "10" sorts before "9"), the first
cutover carried the watermark past a later partition, which was then dropped
as already-archived without ever being exported: rows in neither tier, exit 0.

Candidates now sort by bound. The watermark is read once per cycle instead of
once per partition, so loop order cannot change a partition's outcome.
Dropping a stale partition requires it to be empty rather than relying on the
watermark alone.

The two-level path was unaffected: it already grouped leaves by upper bound
and never consulted the watermark.

Closes #71

[coderabbitai]

Warning

Review limit reached

@vyruss, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 45 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 5111a5c1-237c-4efd-a457-9b6ece6366d2

📥 Commits

Reviewing files that changed from the base of the PR and between 9dd9cba and 0b9396b.

📒 Files selected for processing (6)

• ci/journey.sh
• cmd/archiver/main.go
• cmd/archiver/main_test.go
• docs/architecture_tiered.md
• internal/partition/partition.go
• internal/partition/partition_test.go

📝 Walkthrough

Walkthrough

The archiver now processes partitions by range-bound order, uses one watermark snapshot per cycle, and verifies that stale partitions are empty before removal. Unit tests, architecture documentation, and tiered journey coverage validate these behaviors.

Changes

Bound-ordered archival

Layer / File(s)	Summary
Partition bound ordering internal/partition/partition.go, internal/partition/partition_test.go, docs/architecture_tiered.md	Partition listing now returns row-iteration errors and sorts partitions by ascending lower bound, including MINVALUE. Tests and documentation describe the bound-ordering contract.
Stable watermark and safe cleanup cmd/archiver/main.go, cmd/archiver/main_test.go	Archive cycles reuse one watermark snapshot. Cleanup counts rows before removal, rejects non-empty stale partitions, and detaches and drops only empty partitions. Tests verify both paths.
End-to-end partition safety coverage ci/journey.sh	Tiered journeys validate cold empty-partition removal, MINVALUE handling, bound-based ordering, row preservation, and refusal to drop non-empty stale partitions. The new story runs in the tiered sequence and mesh mode.

Estimated code review effort: 4 (Complex) | ~45 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the primary changes: bound-order partition tiering and refusal to drop non-empty partitions.
Description check	✅ Passed	The description directly explains the ordering defect, fixed watermark handling, and safe cleanup behavior addressed by the changes.
Linked Issues check	✅ Passed	The changes satisfy issue #71 by ordering candidates by bounds, stabilizing the watermark per cycle, and preventing unsafe drops.
Out of Scope Changes check	✅ Passed	The code, tests, journey coverage, and documentation changes all support the linked issue and stated partition-safety objectives.

✨ 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 fix/partition-tier-order

---

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

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 0 complexity · 0 duplication

Metric	Results
Complexity	0
Duplication	0

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.}

[coderabbitai]

🧹 Nitpick comments (1)

internal/partition/partition_test.go (1)

356-390: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add coverage for the rows.Err() propagation path.

listPartitions now returns rows.Err() before sorting (internal/partition/partition.go lines 361-363), but mockRows.Err() always returns nil. No test exercises the case where row iteration fails. Add a case where a mock Err() returns a non-nil error, and assert FindExpired/listPartitions propagates it.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/partition/partition_test.go` around lines 356 - 390, Add a test
covering rows.Err() propagation through FindExpired/listPartitions by
configuring the mock rows returned by mockDB to report a non-nil iteration error
after row processing. Assert the returned error matches the injected error and
that no successful partition result is used; extend the existing mockRows test
setup or add a focused test alongside the FindExpired cases.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@internal/partition/partition_test.go`:
- Around line 356-390: Add a test covering rows.Err() propagation through
FindExpired/listPartitions by configuring the mock rows returned by mockDB to
report a non-nil iteration error after row processing. Assert the returned error
matches the injected error and that no successful partition result is used;
extend the existing mockRows test setup or add a focused test alongside the
FindExpired cases.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 1342e0d5-7d85-4c60-b96b-7d7276762c9a

📥 Commits

Reviewing files that changed from the base of the PR and between cd3f350 and 9dd9cba.

📒 Files selected for processing (6)

• ci/journey.sh
• cmd/archiver/main.go
• cmd/archiver/main_test.go
• docs/architecture_tiered.md
• internal/partition/partition.go
• internal/partition/partition_test.go