Seal RelationType as a sum-type with one parse boundary (links-va-001-relation-type-raq.1)#206
Conversation
…oundary (links-va-001-relation-type-raq.1) Relation.Type was a raw string, forcing eight scattered string-equality defenses across CLI and store. model.RelationType is now the sealed set (RelBlocks/RelParentChild/RelRelatedTo); ParseRelationType is the single string->type gate at trust boundaries (CLI --type flags, import payloads). - StoreEndpoints absorbs the blocks orientation rule (one involution, serves both store and display direction); CanonicalEndpoints absorbs the related-to undirected-sort normalization (was 3 inline copies). - Store-side duplicate validation chains deleted; AddRelationInput and RemoveRelation take the typed value. - ListRelationsForIssue filter is a variadic type set, not a ""-sentinel. - Behavior improvement: dep rm/ls with an unknown --type now fail loudly at parse (exit 3) instead of NotFound / silently matching nothing. - Epic drift of record: legacydolt sites no longer exist; the GetIssueDetail switch was already absorbed into bucketRelations. Verified: go test ./... clean; old-vs-new binary byte-identical on live store reads; blocks/related-to round trips + error paths exercised on a scratch workspace.
There was a problem hiding this comment.
Z.ai Coding Agent Review
This is a clean refactor that introduces RelationType as a sealed sum type with ParseRelationType as the single parse boundary, and moves orientation/normalization logic onto the type as StoreEndpoints/CanonicalEndpoints. It eliminates three independent string-validation chains (CLI, AddRelation, ImportRelation) in favor of one gate, and replaces the sentinel-string ListRelationsForIssue signature with a variadic ...RelationType — all consistent with [LAW:types-are-the-program], [LAW:single-enforcer], and [LAW:one-source-of-truth].
No must-change items found. Two pre-existing / low-priority observations for the author's attention:
-
depRelationLinedefault case (LINE 229) — Now unreachable dead code; the sealed type guarantees only the three named constants flow through, so thedefaultbranch can never fire. Consider removing it so the compiler catches any future gap instead of silently falling through. -
Inconsistent
string()conversion at SQL boundary —ImportRelation(LINE 344) andRemoveRelation(LINE 303) explicitly cast tostring()before passing toExecContext, butAddRelation(LINE 270) passesrel.Typeas the rawRelationType. If the driver handles both (likely, since the test suite presumably passes), the explicit casts in the other two call sites are unnecessary; if the casts are needed, theAddRelationpath has a latent bug. One pattern or the other should be applied consistently.
✅ Approved
Summary
Implements F-11 from the va-001 epic (
links-va-001-relation-type-raq):Relation.Typewas a rawstring, so every layer carried its own string-equality defenses.model.RelationTypeis now the sealed sum (RelBlocks/RelParentChild/RelRelatedTo) andParseRelationTypeis the single string→type gate at trust boundaries ([LAW:types-are-the-program],[LAW:single-enforcer]).ParseRelationType— called at the CLI--typeflags and inImportRelation; preserves the exact legacy error message. Store-side duplicate validation chains are deleted, not kept defensively.StoreEndpoints— the blocks orientation rule (store encodes dependent→dependency, reverse of human order) lives in one involution;depStoreEndpointsanddepRelationForCLI's early-return branch are gone — the flip now applies unconditionally with variability in the value ([LAW:dataflow-not-control-flow]).CanonicalEndpoints— the related-to undirected-sort normalization, previously 3 inline copies (AddRelation / RemoveRelation / ImportRelation), lives in one method ([LAW:one-source-of-truth]).ListRelationsForIssuetakes a variadic type-set filter instead of a""-sentinel string.merge/shapemapupdated; the lifeboat salvage path uses a raw conversion deliberately (conserves bytes; Doctor re-checks) and is marked as the law exception.Drift from the epic (design of record, sites re-located by grep)
internal/legacydoltno longer exists.GetIssueDetailswitch was already absorbed intobucketRelations(PR feat(epic-view): wire epic context into 'lit show' + shared batch relations accessor (links-epic-view-n5o.3, .1.1, .4) #172).ImportRelation(and the whole per-rowImport*API) has zero callers — converted minimally here; deletion filed aslinks-store-cleanup-bex7.schema_reconcile.gois a frozen artifact.Behavior changes (deliberate, [LAW:no-silent-failure])
dep rm --type <junk>now fails at parse (exit 3, documented message) instead of a NotFound for a nonsense key.dep ls --type <junk>now fails at parse instead of silently printing nothing.Verification
go test ./...clean; new pins:TestParseRelationType,TestRelationTypeStoreEndpoints(involution),TestRelationTypeCanonicalEndpoints,TestDepRejectsUnknownRelationType(CLI boundary, all three subcommands).dep lstext/JSON/filters,show,backlog).rm(canonicalization), self-loop rejection, parse error exit codes.