Skip to content

Source: explicit file-carried id, decoupling matching from Title/Type/Date content #162

Description

@DutchJaFO

Background

Today, ResolveSourceAsync (the import planner) matches an incoming Source by its natural key
(Title+Type) and, on a match, silently reuses the existing row's id — it never compares any other
field, including Date. Worse: because Title/Type together are the match key, a corrected title
or type can never be detected as drift on an existing row — it simply fails to match and gets staged
as a brand-new Source instead. Verified directly against EntityIdentity.SourceId/ResolveSourceAsync.

This issue was originally scoped to Date only (Source's one field that isn't part of the natural
key). Scope grew during planning once the same limitation was found to block Title/Type entirely,
and once it became clear that giving Source a real Modify path exposes the same "must never silently
overwrite a reviewed record" problem every future decidable entity will face. That general mechanism
was split out into its own prerequisite issue — this issue depends on it and only consumes it.

Depends on #165 (generalized completeness/review model + ImportActionStatus.Blocked +
whole-batch hold + decide-time MarkCompletenessAs) — must land first; this issue's Modify path
relies on CompletenessGuard, ImportActionStatus.Blocked, and Sources.CompletenessStatus/
NoValueKnown all existing already.

This is the first of what's expected to become several "make entity type X's fields decidable" issues
(Character, Person, Conversation, StageDirection, SoundCue follow later), and the first entity to
combine an explicit file-carried id (the identity model Conversation/StageDirection/SoundCue
already use, #67/#68 — those three are Add-only) with Modify/merge support (Quote's decidability
model) — a new combination, designed here.

Blocks the planned "bulk-decide via file export/import" issue (#163) — its file format is
entity-agnostic already, but has nothing to exercise for non-Quote rows until this lands.

What needs to be done

  1. schemas/source-extended.schema.json gains a sources array (same shape/precedent as
    stageDirections/soundCues/conversations): id (required, UUID-v4 pattern, same regex as the
    other three), title (required), type (required), date (optional). Purely additive — a file
    without a sources section parses identically to today.
  2. New SourceEntry record (Quotinator.Core.Import), doc-commented like SourceStageDirection/
    SourceSoundCue's "assigned at authoring time and never changes." ParsedSourceFile gains
    Sources (defaults []). SourceQuoteFileReader.TryParseExtended gains the new root-key parse.
    SourceQuote.Source (the per-quote title string) is unchanged — converter-driven bundled
    sources (NikhilNamal17, vilaboim) keep working exactly as today, forever.
  3. ImportActionPlanner gains PlanSourcesAsync, run before quotes resolve. For each declared
    SourceEntry: (a) look up by exact id first (new Sql.Sources.SelectExistingById) — if found,
    this Source already uses the explicit-id model, so compare Title/Type/Date freely and stage
    Modify/Blocked/nothing via CompletenessGuard.ShouldBlock; (b) fall back to the existing
    natural-key lookup (Sql.Sources.SelectExistingByTitleAndType) only if no id-match — a
    not-yet-migrated row; Date stays correctable via the same Modify path (natural-key matching
    already finds it), but Title/Type are not yet correctable on it (see scope boundary below); (c)
    no match at all — stage an Add using the file's own SourceEntry.Id, not
    EntityIdentity.SourceId. EntityIdentity.SourceId is not removed — it remains the fallback
    id-assignment mechanism for a Source discovered only implicitly through a quote (no sources
    section, or the section doesn't cover this title/type), preserving 100% backward compatibility for
    converter-driven sources.
  4. Scope boundary, explicit: this issue does not implement automatic re-keying of a pre-existing,
    natural-key-matched row onto a newly authored explicit id — that needs cascading every FK
    (Quotes.SourceId, Characters.SourceId, SourceTranslations.SourceId) atomically, a distinct,
    higher-risk migration deserving its own future issue and review.
  5. Sql.Sources gains SelectExistingById, SelectExistingByTitleAndType, UpdateById (updates
    Title/Type/Date/DateModified/NoValueKnown/CompletenessStatus). SourceActionPayload
    gains Date. EnsureSourceExistsAsync gains a string? date parameter (currently hardcodes
    Date = null even on Add — no path today ever persists a Source Date at all).
  6. ApplyResolvedActionAsync's Source case splits on ActionType: Add → EnsureSourceExistsAsync
    (as today, now passing Date); Modify → deserialize MergedFields, call Sql.Sources.UpdateById,
    then CompletenessGuard.ComputeNextStatus (or the decide-time MarkCompletenessAs override, per
    the prerequisite issue) to persist the resulting CompletenessStatus.
  7. ConflictDecisionRequest gains SourceTitle/SourceType/SourceDate (nullable FieldDecision?
    each — no new endpoint/DTO, existing route unchanged). DecideAsync gains an EntityType == Source branch before the existing != Quote rejection.
  8. ReverseAppliedActionsAsync's Source case currently always soft-deletes regardless of ActionType
    (harmless today — every Source action is an Add — but wrong once Modify exists). Branch on
    ActionType.Parsed: Add keeps today's soft-delete-if-unreferenced behaviour; Modify restores
    the full field set from ExistingValue via Sql.Sources.UpdateById (no reference-count check
    needed — a Modify reversal never deletes anything).
  9. README.md/addon/DOCS.md/Quotinator.slnx updated in the same commit as usual.

Expected tests

Test class Test method Starts
New: Quotinator.Core.Tests TryParseExtended_NoSourcesSection_ParsesIdenticallyToToday ❌
New: Quotinator.Core.Tests TryParseExtended_SourcesSection_ParsesAllFields ❌
New: Quotinator.Engine.Tests PlanSourcesAsync_IdMatchFound_ComparesAllThreeFields ❌
New: Quotinator.Engine.Tests PlanSourcesAsync_NoIdMatch_FallsBackToNaturalKey ❌
New: Quotinator.Engine.Tests PlanSourcesAsync_NoMatchAtAll_StagesAddWithFileId ❌
New: Quotinator.Engine.Tests PlanSourcesAsync_TitleTypeDateDiffer_StagesModifyAction ❌
New: Quotinator.Engine.Tests PlanSourcesAsync_CompleteStatus_StagesBlockedNotModify ❌
New: Quotinator.Api.Tests DecideImportAction_SourceEntityType_AcceptsTitleTypeDateDecisions ❌
New: Quotinator.Engine.Tests ApplyBatchAsync_SourceModify_WritesResolvedFields ❌
New: Quotinator.Engine.Tests ReverseAppliedActionsAsync_SourceModify_RestoresExistingValue ❌
New: Quotinator.Engine.Tests ReverseAppliedActionsAsync_SourceAdd_StillSoftDeletesIfUnreferenced ❌

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