Skip to content

feat(store): delete objects and remember when they were stored - #36

Open
PiquelChips wants to merge 1 commit into
feat/landfrom
feat/collection
Open

PiquelChips wants to merge 1 commit into
feat/landfrom
feat/collection

Conversation

@PiquelChips

@PiquelChips PiquelChips commented Nov 4, 2026 •

Copy link
Copy Markdown
Member

Nothing could delete an object from the store, and nothing knew how long one had been stored. Payload retention, in the next PR, needs both. Deleting must also never race a writer that reuses an object it found already stored.

Deleting:

  • Store::remove deletes objects durably, loose and packed. Each pack holding one is rewritten into a single new pack without them (remove_packed, which shares replace_packs with compaction).
  • Store::reach finds every object some trees refer to, which is what a collector must keep.

When an object was stored:

  • Store::stored_at is when this node first wrote the object: its loose file's modification time.
  • Pack indexes now record that time for each object, so packing never resets it. An index entry grows from 12 to 20 bytes.

Collection never races a writer:

  • A writer that finds an object already stored, and skips writing it, holds the store's objects (Store::hold_objects, a shared lock) until it has written what refers to that object. Issuing a statement and snapshotting the working copy both do.
  • A collector takes the lock exclusively (Store::lock_for_collection).
  • A batch rereads the pack manifest before it trusts a pack to hold an object (Batch::packed), since a collection may have rewritten that pack without it.

This used to be part of #12. The tests that run a real collector against these locks come with the collector, in the next PR.

Review fixes: promotions, in promote and in the queue, hold the shared collection guard until they are published, so collection never deletes a payload a new promotion references. Proved with focused tests that fail without their fix, and just (358 tests).

Reviewed with review-loop: 3 rounds with 2× GPT-6.1 Sol and Claude Opus 5.5. Verdict: approved

Note

Written by Claude Opus 5.5 in Claude Code (via T3 Code)

🤖 Generated with Claude Code

@PiquelChips PiquelChips added enhancement New feature or request area/storage Objects, records, packs and the record index labels Nov 4, 2026
@coderabbitai

coderabbitai Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 29 minutes.

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: 40594171-86ec-425d-b69b-f4e755b4aac5
Commits

Reviewing files that changed from the base of the PR and between df1ef36 and cad0322.

Files selected for processing: 7

  • apps/origin-src/src/annotations/mod.rs
  • apps/origin-src/src/annotations/promotion.rs
  • apps/origin-src/src/queue.rs
  • apps/origin-src/src/store/mod.rs
  • apps/origin-src/src/store/packs.rs
  • apps/origin-src/src/tree.rs
  • apps/origin-src/src/workspace/mod.rs
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@PiquelChips
PiquelChips added this pull request to stack #38 November 4, 2026 18:00
@PiquelChips PiquelChips self-assigned this Nov 4, 2026
@PiquelChips PiquelChips added this to the 0.1 milestone Nov 4, 2026
@github-project-automation github-project-automation Bot moved this to Needs Triage in origin-src.dev Nov 4, 2026
@PiquelChips PiquelChips moved this from Needs Triage to In Progress in origin-src.dev Nov 4, 2026
@PiquelChips
PiquelChips force-pushed the feat/collection branch 2 times, most recently from eaf10c5 to dac8f33 Compare November 4, 2026 18:33
@PiquelChips
PiquelChips force-pushed the feat/collection branch 8 times, most recently from 59d3a74 to df5ed59 Compare November 4, 2026 22:26
@PiquelChips
PiquelChips force-pushed the feat/collection branch 2 times, most recently from 0ebf8db to 3e79923 Compare November 8, 2026 13:57
PiquelChips added a commit that referenced this pull request Nov 8, 2026
Nothing could attach claims to revisions: CI results, reviews, a
recorded abandonment. Nor could a result be carried over to a new
revision whose inputs hadn't changed.

**Commands:**
- `origin annotate` issues a statement about a revision. It supersedes
the user's previous statement of the same kind about that revision. Its
details come inline with `-m "<text>"`, such as a review's message, or
from a file with `--payload <file>`, where `-` reads standard input so a
log can be piped in. `--withdraw` takes the user's statement back: it
supersedes it with one that has no verdict, refused when there's nothing
to take back, and `origin statements` shows it as withdrawn. `--on
<statement>` makes another statement the subject instead of a revision,
such as a `flaky` mark on a failed check, and `origin statements` lists
each statement's own statements indented under it.
- `origin statements` lists the statements about a revision.
- `origin promote` carries promotable results over from records with the
same input digest. Each one becomes a new statement whose
`promoted_from` points at the original.

**Rules:**
- **The input digest** hashes what a snapshot stores at each declared
path, plus the check's definition and toolchain. It costs one lookup per
path.
- **A result that ran there wins.** A promotion stops counting once the
record has a statement of that kind from the original's issuer that
isn't a promotion, a result or a withdrawal, whichever came first.
`Statement::drop_replaced_promotions` holds the rule, so the server and
plugins apply it alike.
- **Promotions are checked against their original.** `Store::claim` only
accepts one whose kind, verdict, inputs and payload match the
original's, about a record with the same input digest. A promotion
counts as its original's issuer.
- **Supersession** only counts within one issuer.
- **Reserved kinds have authors.**
- `core.abandon`: only the change's creator, the author of its first
revisions, or an admin from #6.
  - `core.conflict`: a node, or the revision's author.
  - `core.divergent`: a node.
  - Reserved kinds never promote.

Deleting objects (#36) and payload retention with `origin gc` (#37) used
to be part of this PR. They're now their own.

**Benchmarks:** an input digest costs about 40 µs. Searching 20k
statements for promotions takes 1.2 ms with the digest cache, or 41 ms
without it.

**Review fixes:** `@` names the working copy's own revision, a prefix
matching both a change and another revision is ambiguous, `statements`
lists a promotion whose original is missing, `new` on a stale revision
flags the old change's divergence, and `abandon` checks authorization
before writing anything. Proved with focused tests that fail without
their fix, and `just` (332 tests).

Reviewed with review-loop: 2 rounds with 2× GPT-6.1 Sol and Claude Opus
5.5. Verdict: approved

**Withdrawals:** Reviewed with review-loop: 3 rounds with 2× GPT-6.1 Sol
and Claude Opus 5.5. Verdict: approved

**Statements about statements:** Reviewed with review-loop: 1 round with
2× GPT-6.1 Sol and Claude Opus 5.5. Verdict: approved

> [!NOTE]
> Written by Claude Opus 5.5 in Claude Code, reviewed by GPT-6.1 Sol and
Claude Opus 5.5 over three review rounds (via T3 Code).

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



<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

<!-- coderabbit-ai-generated:v1 -->

## Annotation model and authorization

- Added the public `annotations` module to issue, query, and promote
statements.
- Added `Statement.promoted_from` to identify the source statement for a
promotion. Added checks for reserved kinds and statement promotability.
- Added issuer-scoped supersession. A statement can supersede an earlier
statement only from the same issuer.
- Added authorization rules for reserved kinds:
- `core.abandon` on a revision is limited to the creator of its change,
based on the authors of its root revisions.
- `core.conflict` can be issued by a node, or by the author of the
revision it describes. A node can also issue it about a non-revision
record.
  - `core.divergent` can be issued only by a node.
  - Reserved kinds cannot be promoted.
- Added divergence flagging when a change has multiple heads. The
generated statement uses a deterministic head and includes the sorted
head IDs.
- Added payload storage and retrieval support through `Store::try_blob`.

## Promotion and input digests

- Added promotion discovery and validation. A promotion must reference
an existing promotable statement and match its kind, verdict, inputs,
and payload. The original and target records must have snapshots with
matching digests for the declared input paths.
- Added input-digest computation over sorted, unique input paths. The
digest distinguishes missing entries from present entries and propagates
lookup errors.
- Added index support for promotion groups, statement supersession, and
cached input digests. The index log format is now version 2.
- Added benchmarks for input-digest calculation and promotion searches,
including cached and uncached digest cases.

## CLI and workspace

- Added `origin annotate`, `origin statements`, and `origin promote`.
- `origin annotate` accepts a revision, kind, verdict, optional inputs,
and optional payload. It can supersede the user’s current statement of
the same kind for that revision.
- `origin statements` lists statements and reports verdicts,
supersession, promotion sources, inputs, and payload summaries.
- `origin promote` writes eligible promotions for a revision.
- Added revision-prefix lookup and updated the README usage examples.
- Workspace saves now flag divergence. Abandonment listings now include
only authorized `core.abandon` statements.

## Shared types and tests

- Added `Kind::is_reserved`, `Statement::is_promotable`,
`Statement::promotion_group`, and input specification and snapshot
digest methods.
- Added CLI integration tests for divergence, abandonment authorization,
annotation, supersession, payloads, external inputs, and promotion
behavior.
- Updated statement fixtures and encoded-object expectations for
`promoted_from`.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
@PiquelChips
PiquelChips force-pushed the feat/collection branch 2 times, most recently from 386905d to eb80ceb Compare November 10, 2026 17:22
@PiquelChips
PiquelChips removed this pull request from stack #38 November 13, 2026 22:15
@PiquelChips
PiquelChips added this pull request to stack #75 November 13, 2026 22:15
Nothing could delete an object, and nothing knew how long one had been
stored. `Store::remove` deletes objects durably, loose and packed: each
pack holding one is rewritten into a single new pack without them
(`Store::remove_packed`, which shares `replace_packs` with compaction).
`Store::stored_at` says when this node stored an object: its loose
file's modification time, which pack indexes now record for each object
so packing never resets it.

Deleting must never race a writer that found an object already stored
and skipped writing it. Such writers hold the store's objects
(`Store::hold_objects`, a shared lock) from finding the object to
writing what refers to it, as issuing a statement and snapshotting the
working copy do. A collector takes the lock exclusively
(`Store::lock_for_collection`), and a batch rereads the pack manifest
before it trusts a pack to hold an object (`Batch::packed`), since a
collection may have rewritten that pack without it. `Store::reach`
finds every object some trees refer to, which a collector keeps.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/storage Objects, records, packs and the record index enhancement New feature or request

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

1 participant