specs/015-gerrit-style-patchsets/checklists/revision-model.md

Ref: Size: 6.2 KiB
# Revision Model Requirements Quality Checklist: Patch Revisions

**Purpose**: Validate completeness, clarity, and consistency of requirements for the patch revision system
**Created**: 2026-03-21
**Feature**: [spec.md](../spec.md)

## Requirement Completeness

- [ ] CHK001 - Are requirements defined for what happens when `patch revise` is run on a closed or merged patch? [Completeness, Gap]
- [ ] CHK002 - Is the behavior of `patch merge` with `require_approval_on_latest=true` specified when the patch has zero revisions (legacy patch)? [Completeness, Spec §FR-014]
- [ ] CHK003 - Are requirements defined for how `patch log` handles legacy patches with no recorded revisions? [Completeness, Spec §FR-010]
- [ ] CHK004 - Is the behavior specified for `patch comment --revision N` when N is higher than the current latest revision? [Completeness, Spec §FR-004]
- [ ] CHK005 - Are requirements defined for what `patch diff --between N M` shows when N > M (reversed order)? [Completeness, Spec §FR-006]
- [ ] CHK006 - Is the `refs/collab/config` schema for merge policy specified (format, how to read/write, default when absent)? [Completeness, Spec §FR-014]
- [ ] CHK007 - Are requirements defined for how `patch show --json` includes revision data in its output? [Completeness, Spec §FR-008]

## Requirement Clarity

- [ ] CHK008 - Is "topological DAG order" precisely defined for determining revision numbering? Does it mean oldest-first or newest-first? [Clarity, Spec §FR-003a]
- [ ] CHK009 - Is the scope of "write operations" that trigger auto-detection exhaustively listed, or could `patch close` also trigger it? [Clarity, Spec §FR-002]
- [ ] CHK010 - Is it clear whether `--revision N` on `patch comment` (with `--file`/`--line`) targets revision N's tree for validation, or just records the number without validation? [Clarity, Spec §FR-004]
- [ ] CHK011 - Is the meaning of "latest revision" precisely defined when the branch has moved but no write operation has recorded it yet? [Clarity, Spec §Assumptions]
- [ ] CHK012 - Is it specified whether the `body` field on auto-detected `PatchRevision` events is always `null`, or could it be populated from some default? [Clarity, Spec §Event Schema]

## Requirement Consistency

- [ ] CHK013 - Is the relationship between the existing `PatchRevise` action and the new `PatchRevision` action consistent? The plan says "replaced" but spec says "can be kept as an alias" [Consistency, Spec §Assumptions vs Plan §6]
- [ ] CHK014 - Are the `commit`/`tree` field names on `PatchCreate` consistent with the `branch` field already storing the branch name? Could `commit` conflict with the existing `branch` alias for `head_commit`? [Consistency, Spec §Event Schema]
- [ ] CHK015 - Is `--revision` flag behavior consistent between `patch comment` (rejects for thread comments) and `patch review` (always allowed)? [Consistency, Contracts §CLI]
- [ ] CHK016 - Does the spec's description of `patch show --revision N` filtering align with the contract's definition? Spec says "inline comments and reviews," contract says the same — are thread comments explicitly included or excluded from the filter? [Consistency, Spec §FR-009 vs Contracts §CLI]

## Acceptance Criteria Quality

- [ ] CHK017 - Can SC-001 ("patch revise correctly records revision snapshots") be objectively measured without implementation knowledge? [Measurability, Spec §SC-001]
- [ ] CHK018 - Is SC-007 ("TUI dashboard displays revision count") testable independently, or does it require visual inspection? [Measurability, Spec §SC-007]
- [ ] CHK019 - Are acceptance scenarios for User Story 1 testable for the auto-detection mechanism without race conditions in the test setup? [Measurability, Spec §US-1]

## Scenario Coverage

- [ ] CHK020 - Are requirements defined for the interaction between `patch revise` (explicit) and auto-detection when both happen concurrently from different collaborators? [Coverage, Gap]
- [ ] CHK021 - Are requirements defined for `patch diff --revision N` when the base branch no longer exists or has been renamed? [Coverage, Edge Case]
- [ ] CHK022 - Are requirements specified for how sync handles `PatchRevision` events from a collaborator who has a different branch tip? [Coverage, Spec §FR-012]
- [ ] CHK023 - Is the behavior defined for `patch create` when the branch has uncommitted changes (dirty working tree)? [Coverage, Gap]
- [ ] CHK024 - Are requirements defined for how the TUI displays patches with many revisions (e.g., 50+)? Truncation? Scrolling? [Coverage, Spec §SC-007]

## Edge Case Coverage

- [ ] CHK025 - Is behavior defined when a `PatchRevision` event references a commit OID that no longer exists in the repository (e.g., after garbage collection)? [Edge Case, Gap]
- [ ] CHK026 - Are requirements defined for `patch diff --between 1` (single arg shorthand) when there is only one revision? [Edge Case, Spec §FR-006]
- [ ] CHK027 - Is behavior specified for `patch comment --revision 0` or `patch comment --revision -1` (invalid revision numbers)? [Edge Case, Gap]
- [ ] CHK028 - Are requirements defined for what happens if the tree OID stored in a revision doesn't match the actual tree of the stored commit OID (data corruption)? [Edge Case, Gap]

## Backward Compatibility Requirements

- [ ] CHK029 - Is the deserialization behavior for old `PatchRevise` events explicitly specified alongside the new `PatchRevision` events? [Completeness, Spec §Assumptions]
- [ ] CHK030 - Are requirements defined for what revision number is assigned to legacy inline comments that have `revision: null`? Is "unknown" a display-only concept or does it affect filtering? [Clarity, Spec §FR-011]
- [ ] CHK031 - Is it specified whether `patch show --revision 1` on a legacy patch (no revisions) returns an error or shows all comments? [Coverage, Spec §FR-011]
- [ ] CHK032 - Are requirements defined for the bootstrap behavior when auto-detection creates revision 1 on a legacy patch that already has inline comments with `revision: null`? Do those comments remain "unknown" or get retroactively assigned to revision 1? [Completeness, Gap]

## Notes

- Check items off as completed: `[x]`
- Add comments or findings inline
- Items are numbered sequentially for easy reference
- Focus areas: revision data model, CLI consistency, backward compatibility, auto-detection edge cases