Skip to content

Generalize record completeness to a 3-state model and make it a hard block on modifying completed rows #165

Description

@DutchJaFO

Background

Migration006_RecordCompleteness (#55) added a boolean IsComplete + JSON-array NoValueKnown to
Quotes/Sources/Characters/People, intended to protect a human-confirmed-complete record from
being silently overwritten by a later import. Verified against the actual current code: this
migration has not shipped in any release tag (v1.7.2 is latest) and isn't merged to main yet —
only present on this feature branch, so nothing here needs to preserve compatibility with a released
database. Also verified: nothing in the codebase currently reads either column to make a decision
— they're written once at insert time (hardcoded 0/'[]') and never consulted again.
Quotes.UpdateOnNewestWins excludes them from its SET list, which is the only place they're
referenced at all outside entity property declarations. Conversations/StageDirections/SoundCues
have neither column.

This issue emerged while scoping #162 (Source field decidability): giving Source's Title/Type/
Date a Modify path means, for the first time, the staging engine can attempt to overwrite a field on
a record a human has already reviewed — and that must never happen silently, for the whole batch, not
just the one affected action. This is a general mechanism every future "entity X decidability" issue
needs (Character, Person, Conversation, StageDirection, SoundCue), so it's being built once, generically,
here, rather than reinvented per entity. #162 depends on this issue and only consumes what it builds.

What needs to be done

  1. Replace IsComplete BIT with a 3-state CompletenessStatus enum (Incomplete/NeedsReview/
    Complete) on Quotes/Sources/Characters/People, editing Migration006_RecordCompleteness
    in place (safe — unshipped). Add both CompletenessStatus and NoValueKnown to Conversations/
    StageDirections/SoundCues in the same migration, which have neither column today. Update the
    fresh-database baseline schema in the same commit; add a schema-drift test per the existing
    baseline/incremental-replay convention.
  2. CompletenessStatus lives in Quotinator.Data.Entities (Data-owned, [SafeValue<TEnum?>]-backed
    per ADR 008, registered by the base DatabaseConfiguration) since it's meant to be reused by any
    consuming project's entities, not Quotinator-domain-specific — same pattern as ImportActionStatus/
    ImportBatchStatus. The migrations that add the columns stay in each table's owning project.
  3. New domain-agnostic CompletenessGuard static class in Quotinator.Data.Import (alongside
    FieldMergeResolver): ShouldBlock(CompletenessStatus status, IReadOnlySet<string> changedFields)
    — returns true only when status == Complete; ComputeNextStatus(CompletenessStatus current, IReadOnlyList<string> noValueKnownAfterApply) — computes the Incomplete → NeedsReview
    auto-transition (every field now has a known value, while status was still Incomplete; never
    auto-transitions away from Complete).
  4. New ImportActionStatus.Blocked (Data-owned enum, System_ImportActions.Status CHECK constraint
    widened via a rebuild-under-temp-name migration per database-conventions.md's
    Migration004_ImportBatchTypeUserSeed precedent). An entity planner stages Blocked instead of
    Modify when CompletenessGuard.ShouldBlock returns true for the target row.
  5. ImportActionResolutionCoordinator.TryApplyBatchAsync (Quotinator.Data.Import) widens its
    whole-batch guard from Status == Pending to Status is Pending or Blocked — a batch containing
    even one unresolved Blocked action holds entirely; no action in it applies, including unrelated,
    otherwise-ready actions, until every Blocked action is explicitly decided.
  6. SystemImportAction gains a new nullable, entity-agnostic column: MarkCompletenessAs
    (CompletenessStatus?). Deciding any import action, for any entity type, can optionally set the
    target record's CompletenessStatus directly (most usefully Complete) as part of that same
    decision — always available, not only when resolving a Blocked action. ConflictDecisionRequest
    gains a matching shared MarkCompletenessAs property. SqliteImportActionService.DecideAsync →
    ImportActionResolutionCoordinator.DecideAsync → SystemImportActionWriter.MarkDecidedAsync each
    thread the value through to persist it alongside MergedFields/Status → Decided.
  7. ApplyResolvedActionAsync's per-entity branches read action.MarkCompletenessAs back at apply
    time: if set, write it directly as the row's CompletenessStatus (explicit human override always
    wins, regardless of current status); if not set, fall back to
    CompletenessGuard.ComputeNextStatus(existingRow.CompletenessStatus, computedNoValueKnown). This
    applies to Quote's own existing apply path too, not only to entity types made decidable by later
    issues — Quote already has both columns from Schema: record completeness flag and per-field verified-absent markers #55.
  8. DecideAsync/UndoDecisionAsync/TryReverseBatchAsync behaviour for Blocked otherwise matches
    Pending: deciding a Blocked action is permitted; undo reverts unconditionally to Pending;
    reversing a batch containing an unresolved Blocked action is refused until it's resolved (already
    true today via the existing Status != Applied guard — no code change needed there).

Expected tests

Test class Test method Starts
New: Quotinator.Data.Tests CompletenessGuard_ShouldBlock_OnlyTrueWhenStatusIsComplete ❌
New: Quotinator.Data.Tests CompletenessGuard_ComputeNextStatus_IncompleteToNeedsReviewWhenNoValueKnownEmpty ❌
New: Quotinator.Data.Tests CompletenessGuard_ComputeNextStatus_NeverDemotesFromComplete ❌
New: Quotinator.Data.Tests Migration_BlockedStatus_CheckConstraintAcceptsNewValue ❌
New: Quotinator.Data.Tests Baseline_And_IncrementalReplay_ProduceIdenticalCompletenessSchema ❌
New: Quotinator.Data.Tests TryApplyBatchAsync_BlockedActionInBatch_HoldsEntireBatch ❌
New: Quotinator.Data.Tests TryApplyBatchAsync_BlockedActionResolved_UnrelatedActionsThenApply ❌
New: Quotinator.Engine.Tests ApplyBatchAsync_MarkCompletenessAsProvided_OverridesAutoCompute ❌
New: Quotinator.Engine.Tests ApplyBatchAsync_MarkCompletenessAsOmitted_FallsBackToAutoCompute ❌
New: Quotinator.Engine.Tests ApplyBatchAsync_QuoteAlreadyComplete_MarkCompletenessAsOmitted_StatusUnchanged ❌
New: Quotinator.Api.Tests DecideImportAction_MarkCompletenessAsComplete_PersistsAtApply ❌

Definition of done

  • All expected tests listed above start red before implementation
  • All requirements implemented
  • All expected tests pass (green)
  • No regression in related tests
  • Findings summarised in a closing comment

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions