Skip to content

test(import): pin the checkpoint when the one-by-one fallback stops - #312

Merged
M3gA-Mind merged 1 commit into
tinyhumansai:mainfrom
M3gA-Mind:test/importer-fallback-checkpoint
Oct 23, 2026
Merged

M3gA-Mind merged 1 commit into
tinyhumansai:mainfrom
M3gA-Mind:test/importer-fallback-checkpoint

Conversation

@M3gA-Mind

Copy link
Copy Markdown
Collaborator

Summary

Test only. When the engine refuses a bulk store because of one item, the importer stores that batch one item at a time. If a later item then fails with a non-skippable error (for example the engine going unavailable), the import stops. Its checkpoint must stay on the last item actually stored, so the resume doesn't skip anything. The code does this today, but no test covered the fallback path.

The scripted test engine gets one more mode, DownAfter(n). It refuses every bulk store as invalid, which forces the one-by-one fallback. After n single stores it answers Unavailable.

Related issue

None. Found in an edge-case audit (a coverage gap; the behaviour is correct).

API or behavior changes

None.

Validation

Commands actually run, with their outcome:

  • N/A: cargo fmt --all -- --check. rustfmt --check (edition 2024) passed on the touched file. CI runs the full check.
  • N/A: cargo clippy --all-targets --all-features -- -D warnings. Left to CI.
  • N/A: cargo build --all-targets --all-features. Left to CI.
  • cargo test -p tinymemory-integrations --features legacy-import --test legacy_importer a_failure_in_the_one_by_one_fallback_checkpoints_the_last_stored_item:
    • break-check, with the fallback's fatal branch also setting outcome.checkpoint to the failing item: FAILED, left: Some("d005"), right: Some("d004"). Restored by copy.
    • unchanged code: 1 passed.

Tests

New a_failure_in_the_one_by_one_fallback_checkpoints_the_last_stored_item. With 60 legacy documents and DownAfter(5), 5 items are imported and the checkpoint is d004.

Documentation

Not needed (test only).

Checklist

  • The change is focused on one logical change
  • No new #[allow(...)], #[ignore], or relaxed lints
  • No secrets, tokens, or .env contents in the diff or the description

When the engine refuses a bulk store because of an item, the importer
stores the batch one item at a time. If a later item then fails with a
non-skippable error, the checkpoint must stay on the last item actually
stored. No test covered that path. The scripted engine gains a mode that
refuses every bulk store and goes unavailable after N single stores.
@coderabbitai

coderabbitai Bot commented Oct 22, 2026

Copy link
Copy Markdown

Warning

Review limit reached

  • Run on-demand review

This review includes 1 billable file and costs up to $0.25.

Or wait 54 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: f5832fb3-eeb0-40e0-aa3b-d4ee1addfb5f
📥 Commits

Reviewing files that changed from the base of the PR and between 4f5145c and a2fb398.

📒 Files selected for processing: 1
  • crates/tinymemory-integrations/tests/legacy_importer.rs
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@tinysweeper

tinysweeper Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

Tiny Sweeper review

Reviewed head: a2fb39829ec279fd9f7a327f5a682d171cda4a5f

Verdict: Ready for maintainer review · Priority: none
Coverage: Reviewed 1 of 1 files.
Pull request purpose (title): test(import): pin th…

Diagnostics and supporting details

Full summary

Pull request purpose (title): test(import): pin the checkpoint when the one-by-one fallback stops. Test-only change that adds a `DownAfter(usize)` mode to the scripted test engine and a test pinning the importer's checkpoint when the one-by-one fallback stops on an unavailable engine. No pushed-delta comparison was available.

Review snapshot

Change surface Files Review signal Count
Production 0 Active findings 0
Tests 1 Noted findings 0
Documentation 0 Resolved findings 0
Configuration 0 Pending checks/questions 0

Completeness: Complete
Test assessment: No supported feature-to-test mapping was available; this does not mean tests are absent or passed.

What changed

`Mode::DownAfter(usize)` is added to the test engine, refusing every bulk store as invalid to force the one-by-one fallback and answering `Unavailable` after a configurable number of single stores; `Scripted` gains a `singles` counter for that. A new test imports with 60 legacy documents and `DownAfter(5)`, asserting 5 items stored and the checkpoint at `d004` so a resume cannot skip an unstored item. No production code or API changes.

Features

  • Added — `DownAfter(usize)` scripted engine mode: Lets tests simulate a bulk-store refusal that forces the one-by-one fallback, followed by the engine going unavailable after a set number of single stores. Test-only. (crates/tinymemory-integrations/tests/legacy_importer.rs enum Mode {)
  • Added — Per-engine single-store counter in `Scripted`: Tracks single stores so `DownAfter` can switch to `Unavailable` after the limit; state is per `Scripted` instance, keeping the test deterministic. (crates/tinymemory-integrations/tests/legacy_importer.rs impl Scripted {)

Tests

No supported feature-to-test mapping was produced. Test execution is not inferred.

Findings

No active actionable findings.

Before merge

None.

Agent review details

critique

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: The mode's arms are ordered so bulk-store refusal is only reached for batches larger than one, and the single-store counter only increments on the single-store path, matching the test's expectation; no reportable defect.
  • Lane summary: The change adds a `DownAfter` engine mode and a test asserting the one-by-one fallback checkpoint lags the stored item count. The mode's arms are ordered so that the bulk-store refusal is only reached for batches larger than one, and the single-store counter is only incremented on the single-store path, which matches the test's expectation; I found no reportable defect, but I could not read the importer's `run` body (lookup budget spent), so the assertion that the checkpoint stops at `d004` after five stored items rests on the test's own premise rather than on verified importer code. _The code index is behind this pull request (indexed at `1f4681b009b7`), so retrieved context may be out of date._

security

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: No untrusted input, network calls, or production code touched; the `singles` counter is per-`Scripted` instance and the test is deterministic.
  • Lane summary: This is a test-only change adding a `DownAfter(usize)` engine mode and one new test asserting that a failure in the one-by-one fallback checkpoints the last stored item. No untrusted input, network calls, or production code is touched; the new arm's `singles` counter is per-`Scripted` instance and the test is deterministic. Nothing to report. _The code index is behind this pull request (indexed at `1f4681b009b7`), so retrieved context may be out of date._

tests

  • Conclusion: Neutral
  • Scope reviewed: all assigned evidence
  • Lane summary: No behavioural change: nothing outside documentation, configuration and tests.

commits

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: No sensitive content in committed files.
  • Lane summary: No sensitive content in committed files. _The code index is behind this pull request (indexed at `1f4681b009b7`), so retrieved context may be out of date._

description

  • Conclusion: Success
  • Scope reviewed: all assigned evidence
  • Positive: The new mode and test match the description, which accurately states the fallback checkpoint behaviour is pre-existing and uncovered.
  • Lane summary: Test-only change; the new `DownAfter` mode and the added test match the description, and the description accurately says the fallback's checkpoint behaviour is pre-existing and uncovered. No defects found. _The code index is behind this pull request (indexed at `1f4681b009b7`), so retrieved context may be out of date._
Evidence and run details
  • Models: ladder/vectors-oai3, deepseek-v4.1-flash
  • Spend: $0.000365
  • Tokens: 120004 input · 2265 output · 13312 cached · 129 embedding
  • Updated: 2026-10-22T23:47:15Z
Head State Pass summary
a2fb39829ec2 ready for maintainer review 0 active finding(s), 0 resolved finding(s) (at 2026-10-22T23:47:15Z)

tinysweeper 0.1.2

@tinysweeper tinysweeper Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tinysweeper found nothing blocking. Approving.

Diagnostics

Total spend: $0.0004

@tinysweeper tinysweeper Bot added the priority: p3 Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect. label Oct 22, 2026
@M3gA-Mind
M3gA-Mind merged commit 0638374 into tinyhumansai:main Oct 23, 2026
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: p3 Whenever. Cosmetic, a nicety, or a cleanup with no user visible effect.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant