PR #49. Diff reviewed: git diff origin/main...HEAD (two commits), plus six small uncommitted doc/comment fixes applied during this review.
EntryTeachingDetail.siteMode: SiteMode? — nil means "resolved Site's tuple is illegal"; .untaught still means "no Site row".validatedRecentSiteMode widened private → internal and shared by both screens; the old inline switch/throw block and requireKnownSiteMode call in Entry detail are gone (requireKnownSiteMode stays live on the three write paths).legalSite local gates patterns, work-only flag, actions, replay, display title and hasCurrentURLRule.EntryDetailModel.siteRulesInvalid + an inline amber siteRulesInvalidSection in the view; note/rating stay editable. Unavailability.siteRulesInvalid kept as a defensive fallback.specs/bugfixes/.BootstrapClassifierTests cross-product test failed on one of three make test-core runs; unrelated to this diff.Ready to push
The fix is correct and minimal: EntryTeachingDetail.siteMode becomes optional, entryTeachingDetail(id:) reuses Recent's validatedRecentSiteMode instead of its own weaker throwing checks, and every Site-derived value is withheld on a nil mode — matching Recent's gating exactly. No consumer of siteMode outside EntryDetailModel needed changes; make test-core, make test-quick and make build-ios all pass with zero warnings. Review agents raised no majors in the code; the two majors were stale docs (agent note and implementation.md) still describing the pre-fix behaviour, fixed in the working tree.
8eea496 T-1949: Entry detail tolerates a rule-invalid Site instead of refusing b140755 T-1949: fix stale doc comment on entryTeachingDetail(id:) working-tree Fixes applied in this review (uncommitted) Asterism keeps a per-website record (a Site) of rules for reading titles. Occasionally those rules get into an inconsistent state — for example two active title patterns on one taught site, something a race between two teaches can produce. The app's Recent list already handled this gracefully: it showed the row with a note that its rules need attention. But tapping through to the entry's detail screen threw an error and showed a "Site rules not valid" dead end, so the reader could not see or edit their note and rating.
Now the detail screen loads the entry normally, shows a small amber notice explaining that the site's rules are being repaired, and simply does not offer the teaching actions that depend on those rules.
The entry was never gone — only the site's rules were broken, and the app repairs those on its own. Telling the reader their entry was unavailable was wrong and hid working data.
SiteMode?): a value that may be absent. Here "absent" encodes "the rules did not validate".RecentPresentation.swift: EntryTeachingDetail.siteMode is now SiteMode?.LibraryRepository+RecentPresentation.swift: validatedRecentSiteMode(_:) goes from private to internal.LibraryRepository+EntryDetail.swift: replaces requireKnownSiteMode + a bespoke switch siteMode { throw .quarantined } with a call to validatedRecentSiteMode; introduces legalSite (nil for no-row and for illegal-tuple) and gates six Site-derived values on it.EntryDetailModel.swift: siteRulesInvalid computed property. EntryDetailView.swift: siteRulesInvalidSection reusing the existing Unavailability.siteRulesInvalid.message wording.The design choice is to have one predicate for tuple legality shared by the two read surfaces, so they cannot disagree about the same Site — the bug was precisely that disagreement. The old Entry-detail checks were also weaker than Recent's (no junk-suffix check, no pattern definition decode), so simply catching the throw would have let Entry detail disclose patterns Recent already refused to trust.
validatedRecentSiteMode); rename deferred to keep the diff tight (Decision 13).Unavailability.siteRulesInvalid and its .quarantined mapping are now unreachable for this condition; kept because the view reuses the message text and as a fallback.availableActions == [] for nil mode per Q52 — the diagnostics screen is the only re-teach route; offering Teach would route into a buildTeachingBasis that refuses the hostname.legalSite = siteMode != nil ? site : nil collapses two states (no row, illegal row) into one gate, while siteMode alone still distinguishes them (.untaught vs nil). Consumers that previously read site?.x now read legalSite?.x: patternValues, isWorkOnlyTitleRule, urlRuleValues. availableActions, the cited-pattern replay and displayTitle follow the same gate. A side effect is that try active.definition in the summary builders now only runs on rows whose definitions already decoded inside validatedRecentSiteMode, closing another latent throw path.
The replay gate is worth noting: it withholds replay when the winner's tuple is illegal even if entry.site (a possibly different loser row, Decision 5/9) is legal. This is exactly Recent's if let site, let mode gate, so surfaces agree, but it narrows Decision 9 for this state and Decision 13 does not say so.
Only EntryDetailModel reads EntryTeachingDetail.siteMode, so the optionality change had no ripple; the share extension does not use the DTO. The three write paths (buildTeachingBasis, composed teaching, reparse) still use the throwing requireKnownSiteMode, which is right — refusal is the correct write-side answer (Q52).
hasCurrentURLRule is gated on legalSite though URL rules are not part of the tuple check; defensible, and the field currently has no consumer in the app.performDelete clears entry but not teachingDetail, so siteRulesInvalid can be true after a delete; harmless because the view shows the unavailability screen when entry == nil. Comment reworded to reflect this..notApplicable replay branch has no direct pin.LibraryRepository+EntryDetail.swift
Why it matters. The whole fix. Replaces requireKnownSiteMode plus a weaker inline switch/throw with validatedRecentSiteMode, and introduces the legalSite gate.
What to look at. LibraryRepository+EntryDetail.swift:43-79, 141-149, 172-181, 242-264
RecentPresentation.swift
Why it matters. Public DTO change; nil now carries meaning (illegal tuple) distinct from .untaught (no row).
What to look at. RecentPresentation.swift:219-226, 258-259
EntryDetailView.swift
Why it matters. User-visible behaviour: the entry loads and stays editable; only teaching is withheld.
What to look at. EntryDetailModel.swift:143-157; EntryDetailView.swift:91-96, 482-498
EntryDetailAndMergeToleranceTests.swift
Why it matters. Guards against regressing to the wholesale refusal; asserts each Site-derived field is withheld.
What to look at. EntryDetailAndMergeToleranceTests.swift:77-110
decision_log.md
Why it matters. Records the DTO widening, alternatives and the deferred rename; Q52 row annotated in this review so its citation no longer describes removed behaviour.
What to look at. decision_log.md:45 (Q39), 58 (Q52), 636-673 (Decision 13)
Decision 13. One shared definition of tuple legality; the local checks were weaker (no junk-suffix or definition-decode check).
Q52: the diagnostics screen is the only re-teach route; buildTeachingBasis refuses the hostname, so a Teach button here would be a dead end.
Matches Recent's gate before the same helper. Narrows Decision 9 for this state; not called out in Decision 13 (nit, left for the author).
(inferred — not stated by the author.)Now unreachable from entryTeachingDetail for this condition; kept because the view reuses its message and as a fallback. Decision 13 lists it as a negative consequence.
Decision 13 negative consequence: the name is now inaccurate; rename left for a cleanup pass to keep the diff to the DTO and two call sites.
| Severity | Area | Finding | Resolution |
|---|---|---|---|
| major | docs/agent-notes/testing.md | Note still said "Entry detail refuses outright (Q39)" for a .siteTuple hostname. | Rewrote to describe the Decision 13 behaviour (nil siteMode, inline notice, no Teach action; diagnostics screen still the only re-teach route). |
| major | specs/library-integrity-tolerance/implementation.md | "Partially implemented" table still listed Req 2.1 (Entry detail) as failing wholesale, citing T-1949. | Removed the row. |
| minor | decision_log.md Q52 | Q52's rationale, cited by Decision 13 and the code, still said Entry detail refuses wholesale. | Annotated as superseded by Decision 13. |
| minor | EntryDetailModel.swift siteRulesInvalid comment | Comment said Unavailability.siteRulesInvalid is 'below' (it is above) and claimed teachingDetail and entry are only ever set together (performDelete clears entry alone). | Fixed the direction and reworded the invariant to 'set together in load()', noting the delete path. |
| minor | LibraryRepository+EntryDetail.swift availableActions comment | 'Two states offer none' is stale now that the illegal-tuple paragraph adds a third. | Changed to 'Three'. |
| minor | EntryDetailModelTests.swift quarantinedSiteIsNotReportedAsDeleted | Doc comment narrates the .quarantined throw as the live path and mocks a reason string production no longer emits. | Added a sentence explaining the test now pins the defensive fallback mapping. |
| nit | decision_log.md Decision 13 | "one gate reused four times" — legalSite gates six values. | Reworded to drop the count. |
| minor | EntryDetailAndMergeToleranceTests.swift | The new .notApplicable replay branch has no direct pin (seeded entry has no pattern provenance); only the taught-two-active shape is seeded. | Left for the author: seed a workAssignmentProvenance citing an active pattern and assert assignmentSettlement is the base settlement. The gating path is shared across shapes so one shape is acceptable. |
| minor | EntryDetailModel.swift .quarantined mapping | Mapping in load() is dead for this condition; the view reads a message off an enum named Unavailability for an available entry. | Acknowledged in Decision 13; a shared static message string would be cleaner. Not changed — behavioural refactor outside a doc fix. |
| nit | LibraryRepository+EntryDetail.swift | Same gate spelled three ways (siteMode != nil ? site : nil; if let siteMode, legalSite != nil; if let legalSite, let mode = siteMode). | Could collapse into one if let legalSite, let mode block mirroring Recent. Left as is. |
| nit | validatedRecentSiteMode naming | Now shared with Entry detail; name says Recent. | Deferred per Decision 13. |
| nit | Decision 13 | Does not mention that the replay gate narrows Decision 9 (winner legality over entry.site). | Left for the author. |
| nit | EntryDetailModelTests.swift:120 | #expect(model.teachingDetail?.availableActions.isEmpty == true) asserts the fixture the test built, not behaviour. | Left as is; harmless. |
Click to expand.
diff --git a/Packages/AsterismCore/Sources/AsterismCore/LibraryRepository+EntryDetail.swift b/Packages/AsterismCore/Sources/AsterismCore/LibraryRepository+EntryDetail.swiftindex b96c27a..8c3eb34 100644--- a/Packages/AsterismCore/Sources/AsterismCore/LibraryRepository+EntryDetail.swift+++ b/Packages/AsterismCore/Sources/AsterismCore/LibraryRepository+EntryDetail.swift@@ -7,9 +7,11 @@ extension LibraryRepository { /// Builds an `EntryTeachingDetail` in a single locked context. /// Fetches Entry + Site + retained patterns + assigned Work as needed.- /// Returns exact Site mode, immutable capture evidence, active and historical- /// pattern summaries, independent field settlements, replayed unresolved- /// candidate title, and available actions. No inference or random fallback.+ /// Returns the Site mode (`nil` when the winning row's tuple is illegal,+ /// per Decision 13 of `specs/library-integrity-tolerance`), immutable capture+ /// evidence, active and historical pattern summaries, independent field+ /// settlements, replayed unresolved candidate title, and available actions.+ /// No inference or random fallback. public func entryTeachingDetail(id: UUID) async throws -> EntryTeachingDetail { try await withLockedContext(mode: .shared, operation: "building entry teaching detail") { context in // **Derived, not skipped.** `groupState` below drives@@ -38,13 +40,32 @@ extension LibraryRepository { // hostname (Q12), which is a state every path already handles. let sites = try Self.fetchSites(hostname: hostname, context: context) let site = sites.first- let siteMode: SiteMode+ // **Returns nil rather than throwing** (Q39; see the decision log+ // entry "Entry Detail Tolerates An Illegal Site Tuple, Like Recent").+ // Every arm this used to throw for is the hostname's teaching state+ // failing the closed tuple table — the class `.siteTuple` reports,+ // the class re-teaching clears, and since Decision 7 the class the+ // reconciler repairs on its own. `corruptLibrary` was+ // indistinguishable from a record the app cannot map, and+ // `EntryDetailView` read every one of them as "this entry has been+ // removed" — told the reader their entry was gone while it sat in+ // the store waiting for the next reconciliation pass.+ // `validatedRecentSiteMode` is the one definition of "legal tuple"+ // both screens use, so they cannot disagree about the same Site.+ let siteMode: SiteMode? if let site {- siteMode = try Self.requireKnownSiteMode(of: site, hostname: hostname)+ siteMode = Self.validatedRecentSiteMode(site) } else { siteMode = .untaught } + // Every Site-derived value below is withheld once the tuple is+ // illegal — patterns, presentation title, available actions — the+ // same treatment Recent gives a nil-mode row. `legalSite` is nil for+ // both states that offer no teaching action: no Site row at all+ // (Q40) and a Site row whose tuple does not validate (Q39).+ let legalSite = siteMode != nil ? site : nil+ // Two searches, deliberately (Decision 5). `allPatterns` is the // winning row's own tuple: what the hostname currently teaches, and // what the summaries below disclose. What a provenance replay of an@@ -53,40 +74,9 @@ extension LibraryRepository { // relationship only when there is a citation to replay. A fixed // pointer, so the replay cannot change as unrelated teaching flips // the winner.- let allPatterns = site?.patternValues ?? []+ let allPatterns = legalSite?.patternValues ?? [] let activePatterns = allPatterns.filter(\.isActive)- let isWorkOnly = site?.isWorkOnlyTitleRule ?? false- // **Typed as a quarantine, not as corruption** (Q39). Every arm here- // is the hostname's teaching state failing the closed tuple table —- // the class `.siteTuple` reports, the class re-teaching clears, and- // since Decision 7 the class the reconciler repairs on its own for the- // shape two concurrent teaches produce. The screen refuses either way;- // what the type buys is a caller that can say *why*. `corruptLibrary`- // is indistinguishable from a record the app cannot map, and- // `EntryDetailView` read every one of them as "this entry has been- // removed" — told the reader their entry was gone while it sat in the- // store waiting for the next reconciliation pass.- switch siteMode {- case .untaught where !allPatterns.isEmpty:- throw LibraryRepositoryError.quarantined(- hostname: hostname,- reason: "Untaught Site retains title patterns"- )- case .taught where activePatterns.count != 1:- // A taught Site always retains exactly one active title pattern- // (Decision 5) — whole-title, chapter-less, or ordinary.- throw LibraryRepositoryError.quarantined(- hostname: hostname,- reason: "Taught Site must retain exactly one active title pattern"- )- case .articles where !activePatterns.isEmpty:- throw LibraryRepositoryError.quarantined(- hostname: hostname,- reason: "Articles Site cannot have an active title pattern"- )- default:- break- }+ let isWorkOnly = legalSite?.isWorkOnlyTitleRule ?? false let historicalPatterns = allPatterns .filter { !$0.isActive }@@ -148,9 +138,17 @@ extension LibraryRepository { // reconciled the rows. `SiteReconciler` does, and teaching commits to // the row `SiteResolutionOrder` selects, so the action offered here is // the action the commit performs.- let availableActions = site == nil- ? []- : Self.computeAvailableActions(siteMode: siteMode, isWorkOnly: isWorkOnly)+ //+ // A Site whose tuple is illegal offers none either (Q39, Q52): the+ // diagnostics screen is the only route that can re-teach it, and+ // offering Teach here would route into a `buildTeachingBasis` call+ // that refuses this exact hostname.+ let availableActions: [EntryDetailAction]+ if let siteMode, legalSite != nil {+ availableActions = Self.computeAvailableActions(siteMode: siteMode, isWorkOnly: isWorkOnly)+ } else {+ availableActions = []+ } // An unresolved assignment is replayed with the exact retained pattern // referenced by assignment provenance, never whichever pattern is@@ -171,8 +169,16 @@ extension LibraryRepository { // row, from this same helper — which is also where the rule that an // Entry with no Site relationship has nothing to replay against, and // so is not applicable rather than unresolvable, now lives.- let replay = Self.replayCitedPattern(- for: entrySnap, citingSite: { entry.site })+ //+ // Gated on `legalSite` for the same reason `availableActions` is+ // (Q39): once the winning row's tuple is illegal, nothing it+ // retains is trustworthy enough to replay against, even though+ // `entry.site` — possibly a different row (Decision 5) — might+ // itself be legal. Recent applies the identical gate before calling+ // this same helper.+ let replay: CitationReplay = legalSite != nil+ ? Self.replayCitedPattern(for: entrySnap, citingSite: { entry.site })+ : .notApplicable let assignmentSettlement: FieldSettlement if replay == .unresolvable, let patternID = entrySnap.workAssignmentProvenance.patternID,@@ -233,6 +239,17 @@ extension LibraryRepository { } } + // With no Site row, or a Site row whose tuple is illegal (Q39), there+ // is no trustworthy cleaning or trimming to apply, so the immutable+ // capture title is the presentation title.+ let displayTitle: String+ if let legalSite, let mode = siteMode {+ displayTitle = Self.presentationTitle(+ for: entrySnap.captureTitle, siteMode: mode, site: legalSite)+ } else {+ displayTitle = entrySnap.captureTitle+ }+ return EntryTeachingDetail( entry: entrySnap, siteMode: siteMode,@@ -242,13 +259,9 @@ extension LibraryRepository { assignmentSettlement: assignmentSettlement, availableActions: availableActions, unresolvedCandidateTitle: replay.candidateTitle,- // With no Site row there is no cleaning or trimming to apply, so- // the immutable capture title is the presentation title.- displayTitle: site.map {- Self.presentationTitle(for: entrySnap.captureTitle, siteMode: siteMode, site: $0)- } ?? entrySnap.captureTitle,+ displayTitle: displayTitle, workDisplayTitle: workDisplayTitle,- hasCurrentURLRule: site?.urlRuleValues.contains(where: \.isCurrent) ?? false,+ hasCurrentURLRule: legalSite?.urlRuleValues.contains(where: \.isCurrent) ?? false, groupState: group.state, citingCharacters: citingCharacters )
diff --git a/Packages/AsterismCore/Sources/AsterismCore/RecentPresentation.swift b/Packages/AsterismCore/Sources/AsterismCore/RecentPresentation.swiftindex ef39d5c..fc668a8 100644--- a/Packages/AsterismCore/Sources/AsterismCore/RecentPresentation.swift+++ b/Packages/AsterismCore/Sources/AsterismCore/RecentPresentation.swift@@ -216,8 +216,14 @@ public struct EntryTeachingDetail: Equatable, Sendable { /// two surfaces resolving the same title independently is how they come to /// disagree. public let workDisplayTitle: String?- /// Exact Site mode at read time.- public let siteMode: SiteMode+ /// Site mode at read time, or nil when the resolved Site's committed tuple+ /// is illegal (Q39). Recent resolves the identical condition to nil through+ /// `validatedRecentSiteMode`, which this screen now shares — the two+ /// screens read the same Site and must not disagree about whether its+ /// tuple is legal. A nil mode withholds every other Site-derived value+ /// here (patterns, presentation title, `availableActions`); the record+ /// itself stays fully usable.+ public let siteMode: SiteMode? /// Active pattern summary for this site, if any. public let activePatternSummary: PatternRuleSummary? /// Historical pattern summaries (retained but not active).@@ -250,7 +256,7 @@ public struct EntryTeachingDetail: Equatable, Sendable { public let citingCharacters: [EntryCitingCharacter] public init(- entry: EntrySnapshot, siteMode: SiteMode,+ entry: EntrySnapshot, siteMode: SiteMode?, activePatternSummary: PatternRuleSummary?, historicalPatternSummaries: [PatternRuleSummary], chapterSettlement: FieldSettlement, assignmentSettlement: FieldSettlement,
diff --git a/Packages/AsterismCore/Sources/AsterismCore/LibraryRepository+RecentPresentation.swift b/Packages/AsterismCore/Sources/AsterismCore/LibraryRepository+RecentPresentation.swiftindex 1767363..5d1a2ae 100644--- a/Packages/AsterismCore/Sources/AsterismCore/LibraryRepository+RecentPresentation.swift+++ b/Packages/AsterismCore/Sources/AsterismCore/LibraryRepository+RecentPresentation.swift@@ -290,7 +290,12 @@ extension LibraryRepository { /// the banner that is the only route to the screen listing that very /// diagnosis (Req 4.1). The rows for such a hostname are emitted with /// `.siteRulesInvalid` and no action instead.- private static func validatedRecentSiteMode(_ site: Site) -> SiteMode? {+ ///+ /// **Shared with Entry detail** (`LibraryRepository+EntryDetail.swift`,+ /// Q39): the two screens read the same Site and must not disagree about+ /// whether its tuple is legal, so both call this one definition rather+ /// than each keeping its own partial copy of the closed tuple table.+ static func validatedRecentSiteMode(_ site: Site) -> SiteMode? { // `Site.mode` coerces an unrecognised raw to `.untaught` (Models.swift), // so an illegal raw shows up as an untaught Site retaining patterns; the // explicit parse keeps that from being read as a legal untaught row.
diff --git a/Asterism/Asterism/ViewModels/EntryDetailModel.swift b/Asterism/Asterism/ViewModels/EntryDetailModel.swiftindex b67baa0..7e8da80 100644--- a/Asterism/Asterism/ViewModels/EntryDetailModel.swift+++ b/Asterism/Asterism/ViewModels/EntryDetailModel.swift@@ -29,9 +29,14 @@ public final class EntryDetailModel { /// The record is gone: the reader deleted it here, or another device did /// and the deletion arrived. case deleted- /// The site's teaching state failed validation, so the detail refuses- /// (Q39). Not damage the reader has to act on — the reconciler repairs- /// this class on its own (Decision 7), which is what the message says.+ /// The site's teaching state failed validation. Not damage the reader+ /// has to act on — the reconciler repairs this class on its own+ /// (Decision 7), which is what the message says. `load()` no longer+ /// produces this case (T-1949): a rule-invalid Site now yields a nil+ /// `siteMode` instead of a thrown `.quarantined`, so the record loads+ /// and `EntryDetailModel.siteRulesInvalid` marks it inline rather than+ /// refusing the screen. Kept as a defensive fallback in case+ /// `.quarantined` is ever thrown for this record from elsewhere. case siteRulesInvalid /// A surface refused because this record — or one on its hostname — /// holds differing copies (Req 2.8, Q73). Workload with a route, not@@ -136,6 +141,21 @@ public final class EntryDetailModel { teachingDetail?.citingCharacters ?? [] } + /// Whether this entry's Site retains rules that failed validation (Q39,+ /// T-1949). This used to fail the whole screen — `.quarantined` was thrown+ /// out of `entryTeachingDetail` and this Entry looked deleted (`Unavailability+ /// .siteRulesInvalid` below still exists to catch that if it ever happens+ /// again). The repository now resolves a nil `siteMode` for exactly this+ /// condition instead, the same demotion `validatedRecentSiteMode` already+ /// gave Recent, so the record loads and stays editable; only what the Site+ /// claims to teach is withheld. `teachingDetail?.siteMode == nil` cannot mean+ /// "not loaded yet" here, because `teachingDetail` and `entry` are only ever+ /// set together.+ public var siteRulesInvalid: Bool {+ guard let teachingDetail else { return false }+ return teachingDetail.siteMode == nil+ }+ // MARK: - What the title card says (Q51) /// The heading: the parsed chapter, falling back to the presentation title
diff --git a/Asterism/Asterism/Views/EntryDetailView.swift b/Asterism/Asterism/Views/EntryDetailView.swiftindex ef4242f..7d2680c 100644--- a/Asterism/Asterism/Views/EntryDetailView.swift+++ b/Asterism/Asterism/Views/EntryDetailView.swift@@ -88,6 +88,12 @@ struct EntryDetailView: View { // rating inside it and for every disabled control below it. if model.isReadOnly { duplicateReviewSection } + // T-1949: a rule-invalid Site used to blank this whole screen. The+ // record loads like any other now; this is the inline notice that+ // tells the reader why no teaching action is offered below, without+ // gating the note or the rating the way `duplicateReviewSection` does.+ if model.siteRulesInvalid { siteRulesInvalidSection }+ Section { // The note is what the reader came to write, so the editor gets // room to work in rather than the three lines it used to show.@@ -473,6 +479,24 @@ struct EntryDetailView: View { } } + /// Req 3.4 on this screen (T-1949, Q39): say plainly why no Teach/Re-teach+ /// action is offered below, without touching editing — unlike+ /// `duplicateReviewSection`, nothing here is read-only. The reconciler+ /// clears this on its own; the diagnostics screen is the only manual route+ /// (Q52), so this notice offers no action either.+ private var siteRulesInvalidSection: some View {+ Section {+ Text(EntryDetailModel.Unavailability.siteRulesInvalid.message)+ .font(.callout)+ .foregroundStyle(AsterismColors.primaryText)+ .frame(maxWidth: .infinity, alignment: .leading)+ .padding(12)+ .constellationCard(borderColor: AsterismColors.attentionBorder)+ .constellationListRow()+ .accessibilityIdentifier("entry-detail-site-rules-invalid-notice")+ }+ }+ /// The alert body: the count and one line per copy, both taken from the /// value the alert was built with, which is the disclosure itself. private func disclosureMessage(
diff --git a/Packages/AsterismCore/Tests/AsterismCoreTests/EntryDetailAndMergeToleranceTests.swift b/Packages/AsterismCore/Tests/AsterismCoreTests/EntryDetailAndMergeToleranceTests.swiftindex 7db3669..f2cedeb 100644--- a/Packages/AsterismCore/Tests/AsterismCoreTests/EntryDetailAndMergeToleranceTests.swift+++ b/Packages/AsterismCore/Tests/AsterismCoreTests/EntryDetailAndMergeToleranceTests.swift@@ -74,15 +74,19 @@ struct EntryDetailAndMergeToleranceTests { #expect(detail.availableActions == [.teach]) } - /// The refusal Q39 describes, typed for what it is. A hostname whose rules- /// really are illegal — the field shape: one row carrying two version-1 active- /// title patterns, the union of two concurrent teaches — still refuses this- /// screen, but it refuses as a *quarantine*, naming the hostname. It used to- /// throw `corruptLibrary`, which the detail model could not tell apart from a- /// deleted record, so the reader was told the entry had been removed while it- /// sat in the store waiting for the reconciler.- @Test("Entry detail refuses a hostname whose rules are illegal, naming the quarantine")- func entryDetailRefusesAnIllegalTupleAsAQuarantine() async throws {+ /// T-1949: this used to be the refusal Q39 describes — a hostname whose+ /// rules really are illegal (the field shape here: one row carrying two+ /// version-1 active title patterns, the union of two concurrent teaches)+ /// threw `.quarantined` and lost the whole screen, while Recent already+ /// degraded the identical condition to a marked row with no mode+ /// (`validatedRecentSiteMode`, task 15). The asymmetry is Q39's whole+ /// point: the same broken Site rendered fine in Recent and blanked Entry+ /// detail. This now applies the same treatment Recent already had:+ /// `siteMode` resolves to nil rather than throwing, and everything the+ /// Site would have taught is withheld rather than disclosed — the record+ /// itself, and the reader's ability to edit it, are untouched.+ @Test("Entry detail tolerates a hostname whose rules are illegal instead of refusing the screen")+ func entryDetailToleratesAnIllegalTupleInsteadOfRefusing() async throws { let library = try ToleranceFixture() let entryID = UUID() try library.seed { store in@@ -95,12 +99,19 @@ struct EntryDetailAndMergeToleranceTests { } let repository = try await library.openForApp() - await #expect(throws: LibraryRepositoryError.quarantined(- hostname: "collided.example",- reason: "Taught Site must retain exactly one active title pattern"- )) {- try await repository.entryTeachingDetail(id: entryID)- }+ let detail = try await repository.entryTeachingDetail(id: entryID)++ #expect(detail.siteMode == nil)+ // Site-shaped presentation is withheld, not disclosed from an illegal+ // tuple: no pattern summary, no title cleaning, no teaching action —+ // the diagnostics screen is the only repair route (Q52).+ #expect(detail.activePatternSummary == nil)+ #expect(detail.historicalPatternSummaries.isEmpty)+ #expect(detail.availableActions.isEmpty)+ #expect(!detail.hasCurrentURLRule)+ #expect(detail.displayTitle == "A Work - Chapter 1")+ // The record itself is unaffected: the entry is disclosed, not refused.+ #expect(detail.entry.id == entryID) } /// Q12: a hostname with no Site row is an untaught hostname, which every path
diff --git a/Asterism/AsterismTests/EntryDetailModelTests.swift b/Asterism/AsterismTests/EntryDetailModelTests.swiftindex 33874d4..aeba50e 100644--- a/Asterism/AsterismTests/EntryDetailModelTests.swift+++ b/Asterism/AsterismTests/EntryDetailModelTests.swift@@ -480,6 +480,43 @@ struct EntryDetailModelTests { #expect(model.unavailability.title != "Entry deleted") } + /// T-1949: the repository no longer throws `.quarantined` for this+ /// condition — it resolves `siteMode` to nil instead (Q39), the same+ /// demotion Recent already had. The screen must load the entry rather than+ /// refuse it, and `siteRulesInvalid` is what tells the view to show the+ /// inline notice instead of a Teach/Re-teach action.+ @Test("A rule-invalid Site loads the entry and marks it as needing attention, not refused")+ @MainActor func ruleInvalidSiteLoadsInsteadOfRefusing() async {+ let entry = TestFixtures.makeEntry()+ let detail = EntryTeachingDetail(+ entry: entry,+ siteMode: nil,+ activePatternSummary: nil,+ historicalPatternSummaries: [],+ chapterSettlement: .unsettled(reason: "test"),+ assignmentSettlement: .unsettled(reason: "test"),+ availableActions: [],+ unresolvedCandidateTitle: nil+ )+ let (model, _, _) = makeSUT(entry: entry, detail: detail)++ await model.load()++ #expect(model.entry != nil)+ #expect(model.state == .ready)+ #expect(model.siteRulesInvalid)+ #expect(model.teachingDetail?.availableActions.isEmpty == true)+ }++ @Test("A resolved Site does not mark the entry as needing attention")+ @MainActor func resolvedSiteIsNotFlaggedAsInvalid() async {+ let (model, _, _) = makeSUT()++ await model.load()++ #expect(!model.siteRulesInvalid)+ }+ @Test("An entry that no longer exists still reports as deleted") @MainActor func missingEntryIsReportedAsDeleted() async { let (model, mock, _) = makeSUT()
diff --git a/specs/library-integrity-tolerance/decision_log.md b/specs/library-integrity-tolerance/decision_log.mdindex a586dea..69d2393 100644--- a/specs/library-integrity-tolerance/decision_log.md+++ b/specs/library-integrity-tolerance/decision_log.md@@ -42,7 +42,7 @@ | Q36 | 2026-07-26 | Req 2.2's "missing referenced Work" is given its only reachable form: `recentWorkTitles` omits a Work with a blank display title instead of throwing | The cause as written cannot arise through `Entry.work` in a local store — the relationship is nil or points at a live row. Without this the requirement names a state nothing can produce, while a blank Work title (which only quarantines, per Q27) would still fail Recent wholesale. Does not weaken the fail-closed boundary, which is the validator and `snapshot`. Task 34 ("Write the scale tests for the tolerated states") writes regressions over this state | | Q37 | 2026-07-26 | `LibraryRepository.diagnostics` is seeded at open from the existing `validateV4Store` call, rather than `recentPresentation` running a scan | design.md says `diagnosisCount` is "built in the read", but no diagnosis derivation exists until task 23. Scanning inside `recentPresentation` would double the scan on every foreground once task 29 wires `refreshDiagnostics()` immediately before `refreshAll()`. Task 23 is correspondingly smaller: the property and its threading exist, and it needs only `refreshDiagnostics()` and the public accessor | | Q38 | 2026-07-26 | Attention precedence on a Recent row is `siteMissing` → `siteRulesInvalid` → `workMissing` | The design gives one `attention` field and no ordering for a row that is both site- and work-unresolvable. Site causes rank first because they also explain why the row has no mode and therefore no action. `isActionable` is false when `siteMode` is nil, so attention rows do not inflate `actionableCount` — task 28 must drive the amber edge from `attention != nil`, not from `isActionable` |-| Q39 | 2026-07-26 | Entry detail still fails wholesale for a single Site row with an illegal tuple; Recent now degrades for the same condition | `+EntryDetail.swift:43-62` is the exact analogue of `validatedRecentSiteMode`, which task 15 demoted, and task 17 does not name it. `.siteTuple` is not a Req 1.1 state, so Req 2.1 does not strictly reach it, and fixing it means widening `EntryTeachingDetail.siteMode` to optional — a DTO change no task authorises. Left as-is deliberately, recorded because the asymmetry is now visible and someone will otherwise read it as an oversight. Raise it as its own task if the inconsistency is judged to matter |+| Q39 | 2026-07-26 | Entry detail still fails wholesale for a single Site row with an illegal tuple; Recent now degrades for the same condition | promoted to Decision 13 (T-1949) | | Q40 | 2026-07-26 | Teaching a `.siteMissing` hostname does **not** create the Site row; capture and `createWork` do, so the diagnosis is self-healing through them | Both basis builders refuse a hostname with no Site row today (`+Contracts.swift:14` and `+ComposedTeaching.swift:366`, `invalidInput "no Site exists for hostname"`), while `LibraryRepository.capture` (`:313`) and `createWork` (`:414`) insert one when `fetchSites` comes back empty. So the reader already has a route — capture anything from the hostname and the orphaned Entries reunite with a Site row — and it is the route that matches what a `.siteMissing` hostname actually is: untaught, not broken. Making teaching create the row would add a Site-creating side effect to a commit whose whole contract is "rewrite this Site's tuple", in the phase Decision 6 names as the milestone's largest regression risk, for a state that already heals. Consequence: Req 3.1 does not cover `.siteMissing`, `clearableByReteaching` stays false for it, and Q39's `availableActions == []` and task 15's `actionType == .none` are both correct as they stand | | Q41 | 2026-07-26 | The four write-path guards test `diagnostics.diagnoses` for `.duplicateSiteRows`, not the quarantine map | The map holds one reason per hostname and `.siteTuple` wins when a hostname carries both (Q24), so a duplicated *and* tuple-invalid hostname would be invisible in the projection and the refusal would silently not fire. Reading the diagnosis list also keeps the check narrow by construction: `.siteTuple` must not refuse — it is the class re-teaching exists to clear | | Q42 | 2026-07-26 | `commitTeaching` and `commitArticles` inherit their Req 3.4 refusal from `buildTeachingBasis` rather than carrying a second copy | design.md lists four call sites, but three of them funnel through one builder: `commitTeaching` and `commitArticles` both call `buildTeachingBasis` before any write, so one check covers the two teaching projections, both articles paths, and both commits. A duplicated guard would be two places to keep in step for no behavioural gain. Comments at both commit sites say where the refusal lands; `WritePathQuarantineTests` asserts each entry point separately so a refactor that drops the builder call fails a test |@@ -798,3 +798,43 @@ Req 7.1, and `ComposedRecalculationTests` — which pins both halves with a stal Entry twin and a stale Work twin behind an in-sync representative. ---++## Decision 13: Entry Detail Tolerates An Illegal Site Tuple, Like Recent++**Date**: 2026-08-29+**Status**: accepted++### Context++Task 15 of this milestone demoted `validatedRecentSiteMode` to return nil for a Site whose committed tuple fails the closed table, instead of throwing — Recent renders the row marked as needing attention, with no mode and no action, rather than failing the whole publication. Task 17 did not name Entry detail's own copy of the same checks (`LibraryRepository+EntryDetail.swift`), which kept throwing `.quarantined`. Q39 recorded the asymmetry deliberately rather than fixing it quietly: `EntryTeachingDetail.siteMode` was a non-optional `SiteMode`, and widening it to optional was a DTO change no task had authorised.++The asymmetry became a real bug (T-1949) rather than a documented gap: a reader who tapped through from a working Recent row — the same row Q39 says renders fine, marked as needing attention — hit a dead Entry detail screen for the identical Site. Nothing about the record was actually gone; only the Site's stored rules were, in the vocabulary this codebase already uses, in a state that fails the closed tuple table and is `.siteTuple`-diagnosed and reconciler-clearable (Decision 7).++### Decision++`EntryTeachingDetail.siteMode` becomes `SiteMode?`. `entryTeachingDetail(id:)` no longer throws `.quarantined` for an illegal tuple; it calls the same `validatedRecentSiteMode` Recent already uses (widened from `private` to internal, so both files can call the one definition) and gets nil back. Every other Site-derived value in the DTO — `activePatternSummary`, `historicalPatternSummaries`, `displayTitle`'s cleaning/trimming, `hasCurrentURLRule`, the cited-pattern replay, and `availableActions` — is withheld (empty/nil/raw) whenever the resolved mode is nil, exactly as Recent withholds its own mode-dependent presentation for a nil-mode row. `availableActions` is `[]` for a nil mode, per Q52: the diagnostics screen is the only route that can re-teach a genuinely tuple-diagnosed hostname, and offering Teach here would route into a `buildTeachingBasis` call that refuses the same hostname. `EntryDetailModel` gains `siteRulesInvalid` (true when `teachingDetail?.siteMode == nil`), and the view shows an inline amber notice reusing `Unavailability.siteRulesInvalid`'s existing wording — the note, rating, and delete stay fully editable; only the Site's own teaching is in question.++### Rationale++Reusing `validatedRecentSiteMode` rather than writing a second definition of "legal tuple" is the point, not an implementation convenience: the bug being fixed is exactly two screens disagreeing about the same Site, and two independently-maintained copies of the same closed-table check is how that kind of drift starts. One shared function makes the two screens structurally unable to disagree about legality, the same reasoning already applied to `duplicatedHostnames(in:)` and `replayCitedPattern`.++Withholding every Site-derived field rather than only suppressing the throw matters because the old per-check throws were weaker than `validatedRecentSiteMode` (they omitted the junk-suffix-rule check and the pattern-version/definition validity loop). Falling back to "catch the throw and show something anyway" while leaving the legality definition as it stood would have let Entry detail disclose patterns Recent would already refuse to trust for the same Site — the exact cross-surface disagreement Decision 5 and Req 3.2 exist to prevent elsewhere in this file.++### Alternatives Considered++- **Catch `.quarantined` locally in `entryTeachingDetail` and build a degraded DTO from it**: keeps `requireKnownSiteMode` and the existing per-check throws as the source of truth - Rejected: it perpetuates a second, weaker definition of "legal tuple" than Recent's, which is the drift that produced Q39's asymmetry in the first place, and it does not gain anything `validatedRecentSiteMode` does not already provide.+- **Add a `RecentRowAttention`-shaped enum to `EntryTeachingDetail`**: full parity with Recent's `attention` field - Rejected as more DTO surface than the fix needs. The only new fact Entry detail needs to expose is "the Site's rules did not validate," which `siteMode == nil` already states without a second vocabulary; `EntryDetailModel.siteRulesInvalid` derives it in one line.+- **Leave `availableActions` non-empty for a nil mode, since the pre-fix code offered actions for a duplicated hostname (Q39)**: symmetry with the duplicated-hostname carve-out - Rejected per Q52: a duplicated hostname resolves a real mode that a teaching commit can act on, but a nil mode is exactly the state `buildTeachingBasis` refuses outright. Offering Teach here would be the dead end Req 3.4 exists to prevent, not a working action.++### Consequences++**Positive:**+- The bug is closed: a reader who taps through from a marked Recent row now sees the same entry, editable, with a plain inline explanation, instead of a blank screen.+- Recent and Entry detail cannot drift on "is this Site's tuple legal" again without a compiler-visible change, since both call `validatedRecentSiteMode`.+- The fix reads as a straightforward widening rather than a new subsystem: no new error case, no new enum, one optional field and one gate reused four times.++**Negative:**+- `validatedRecentSiteMode`'s name still says "Recent," which is no longer accurate now that Entry detail calls it too; renaming was left out of this change to keep the diff to the DTO and the two call sites, and is a candidate for a later cleanup pass.+- `Unavailability.siteRulesInvalid` and its `.quarantined` mapping in `EntryDetailModel` are now dead code on the ordinary path — `entryTeachingDetail` cannot produce that throw for this condition any more — and are kept only as a defensive fallback. A future reader may reasonably ask why the case still exists.++---
diff --git a/specs/bugfixes/entry-detail-fails-wholesale-for-rule-invalid-site/report.md b/specs/bugfixes/entry-detail-fails-wholesale-for-rule-invalid-site/report.mdnew file mode 100644index 0000000..06b6b1f--- /dev/null+++ b/specs/bugfixes/entry-detail-fails-wholesale-for-rule-invalid-site/report.md@@ -0,0 +1,198 @@+# Bugfix Report: Entry Detail Still Fails Wholesale For A Rule-Invalid Site++**Date:** 2026-08-29+**Status:** Fixed++## Description of the Issue++A single Site row whose stored rules are internally inconsistent (an illegal+`.siteTuple`, e.g. two active title patterns on one taught row) failed the+**entire** Entry detail screen. Recent already degraded gracefully for the+identical condition — the row is rendered marked as needing attention, with no+mode and no teaching action, rather than the whole publication failing.++**Reproduction steps:**+1. Teach a hostname, then get its Site row into an illegal tuple state (e.g.+ two concurrently-taught active title patterns landing on one row — the+ shape a race between two teaches, or a sync merge, can produce).+2. Open Recent: the affected row renders, marked `.siteRulesInvalid`, with no+ mode and no action.+3. Tap through to that row's Entry detail screen.+4. Observe the screen refuses entirely — `ContentUnavailableView` with a+ "Site rules not valid" message and no way to read or edit the note/rating —+ even though the underlying Entry record is intact.++**Impact:** A reader who taps through from a perfectly working Recent row+lands on a dead Entry detail screen for the same record. The note and rating+are unreachable until the reconciler clears the diagnosis on its own; nothing+was actually lost, but the screen could not say so.++## Investigation Summary++- **Symptoms examined:** `LibraryRepository+EntryDetail.swift` threw+ `LibraryRepositoryError.quarantined` for the same three illegal-tuple shapes+ `LibraryRepository+RecentPresentation.swift`'s `validatedRecentSiteMode`+ already tolerated by returning `nil`.+- **Code inspected:** `entryTeachingDetail(id:)` (the throwing guards at the+ top, `requireKnownSiteMode`, and a second inline `switch siteMode` block+ duplicating a weaker version of the same legality check), `recentPresentation+ (calendar:)` and `validatedRecentSiteMode` (the tolerant analogue, from+ `library-integrity-tolerance` task 15), `EntryDetailModel.load()` and+ `Unavailability` (how the thrown error surfaced as a whole-screen refusal),+ and `EntryDetailView.swift` (how `Unavailability` renders).+- **Hypotheses tested:** Confirmed via `specs/library-integrity-tolerance/decision_log.md`+ Q39 that this asymmetry was already known and deliberately left as a DTO+ change no task had authorised — task 17 (which fixed the throwing behaviour+ elsewhere) did not name Entry detail's copy of these checks. Q52/Q53+ established that the diagnostics screen is the *only* repair route for a+ genuinely tuple-diagnosed hostname, and that Entry detail must not offer a+ Teach/Re-teach action that would be refused for the same reason.++## Discovered Root Cause++`EntryTeachingDetail.siteMode` was a non-optional `SiteMode`, so+`entryTeachingDetail(id:)` had no way to represent "the resolved Site's tuple+does not validate" other than throwing. It threw via `requireKnownSiteMode`+(unrecognised raw mode) and via a bespoke `switch siteMode { ... }` block that+re-implemented (more weakly than `validatedRecentSiteMode`) the same+untaught/taught/articles legality checks Recent already had a tolerant version+of.++**Defect type:** Missing tolerance — a throwing guard left over from before+task 15/17 of `library-integrity-tolerance` demoted Recent's equivalent check,+combined with a DTO whose type could not express the tolerant answer.++**Why it occurred:** Task 15 demoted `validatedRecentSiteMode` to return nil;+task 17 fixed the other throwing guards this milestone found, but did not name+Entry detail's copy, and fixing it required widening a public DTO field —+recorded deliberately as Q39 rather than done quietly, since no task+authorised the DTO change at the time.++**Contributing factors:** The two files had **two independent copies** of "is+this Site's tuple legal" — Recent's tolerant `validatedRecentSiteMode` and+Entry detail's own (weaker, throwing) inline checks — which is exactly the+kind of duplication that produces cross-surface disagreement.++## Resolution for the Issue++**Changes made:**+- `Packages/AsterismCore/Sources/AsterismCore/RecentPresentation.swift` —+ `EntryTeachingDetail.siteMode` widened from `SiteMode` to `SiteMode?`; init+ parameter updated to match.+- `Packages/AsterismCore/Sources/AsterismCore/LibraryRepository+RecentPresentation.swift` —+ `validatedRecentSiteMode` widened from `private` to internal so both files+ can call the one definition of "legal tuple."+- `Packages/AsterismCore/Sources/AsterismCore/LibraryRepository+EntryDetail.swift` —+ `entryTeachingDetail(id:)` now calls `validatedRecentSiteMode` instead of+ `requireKnownSiteMode` plus the inline `switch`/throw block. A `legalSite`+ local (nil for "no Site row" and for "Site row, illegal tuple" alike) gates+ every Site-derived value: `allPatterns`/`activePatterns`/`isWorkOnly`,+ `availableActions` (empty for a nil mode, per Q52), the cited-pattern+ `replay` (`.notApplicable` for a nil mode, matching Recent's identical gate),+ `displayTitle` (falls back to the raw capture title, no cleaning/trimming),+ and `hasCurrentURLRule`.+- `Asterism/Asterism/ViewModels/EntryDetailModel.swift` — new+ `siteRulesInvalid` computed property (`teachingDetail?.siteMode == nil`);+ `Unavailability.siteRulesInvalid`'s doc comment updated to note it is now a+ defensive fallback rather than the normal path for this condition.+- `Asterism/Asterism/Views/EntryDetailView.swift` — new+ `siteRulesInvalidSection`, an inline amber notice (reusing+ `Unavailability.siteRulesInvalid.message`) shown when+ `model.siteRulesInvalid`, that does not gate editing (unlike+ `duplicateReviewSection`).+- `specs/library-integrity-tolerance/decision_log.md` — Q39 marked "promoted+ to Decision 13"; Decision 13 records the DTO change, its rationale, and the+ alternatives considered.++**Approach rationale:** Reusing `validatedRecentSiteMode` rather than writing+a second definition makes the two screens structurally unable to disagree+about tuple legality again — the bug being fixed was exactly that kind of+disagreement. Withholding every Site-derived field (not just suppressing the+throw) matters because the old inline checks were weaker than+`validatedRecentSiteMode` (missing the junk-suffix-rule and+pattern-version/definition checks); a naive "catch and degrade" fix would have+let Entry detail disclose patterns Recent already refuses to trust for the+same Site.++**Alternatives considered:**+- Catch `.quarantined` locally and build a degraded DTO from it, keeping the+ existing (weaker) per-check throws as the source of truth — rejected, this+ perpetuates the duplicated/weaker legality definition that caused the+ asymmetry.+- Add a `RecentRowAttention`-shaped enum to `EntryTeachingDetail` for full+ parity with Recent — rejected as more surface than needed; `siteMode == nil`+ already states the one new fact, and `EntryDetailModel.siteRulesInvalid`+ derives it in one line.+- Keep `availableActions` non-empty for a nil mode, for symmetry with the+ duplicated-hostname carve-out that also keeps Q39's Teach action — rejected+ per Q52: a duplicated hostname resolves a real, actionable mode, but a nil+ mode is exactly the state `buildTeachingBasis` refuses outright.++## Regression Test++**Test file:** `Packages/AsterismCore/Tests/AsterismCoreTests/EntryDetailAndMergeToleranceTests.swift`+**Test name:** `entryDetailToleratesAnIllegalTupleInsteadOfRefusing` (replaces+the old `entryDetailRefusesAnIllegalTupleAsAQuarantine`, which pinned the bug)++**What it verifies:** Given a Site with two concurrently-active title patterns+(an illegal tuple), `entryTeachingDetail(id:)` no longer throws. It returns a+detail with `siteMode == nil`, empty `availableActions`,+`activePatternSummary == nil`, empty `historicalPatternSummaries`,+`hasCurrentURLRule == false`, the raw capture title as `displayTitle`, and the+Entry itself still present and identified.++**Additional tests:**+- `Asterism/AsterismTests/EntryDetailModelTests.swift`:+ `ruleInvalidSiteLoadsInsteadOfRefusing` — a nil `siteMode` from the+ repository loads the entry (`state == .ready`, `entry != nil`) and sets+ `model.siteRulesInvalid`, instead of the model reporting an error/deletion.+ `resolvedSiteIsNotFlaggedAsInvalid` — the ordinary path stays unflagged.++**Run command:** `make test-core` (core-level test) and `make test-quick`+(model-level tests)++## Affected Files++| File | Change |+|------|--------|+| `Packages/AsterismCore/Sources/AsterismCore/RecentPresentation.swift` | `EntryTeachingDetail.siteMode` widened to `SiteMode?`; doc comments updated |+| `Packages/AsterismCore/Sources/AsterismCore/LibraryRepository+RecentPresentation.swift` | `validatedRecentSiteMode` access widened from `private` to internal |+| `Packages/AsterismCore/Sources/AsterismCore/LibraryRepository+EntryDetail.swift` | Illegal-tuple guards no longer throw; site-derived fields gated on tuple legality |+| `Asterism/Asterism/ViewModels/EntryDetailModel.swift` | New `siteRulesInvalid` computed property; doc comment updates |+| `Asterism/Asterism/Views/EntryDetailView.swift` | New inline `siteRulesInvalidSection` notice |+| `Packages/AsterismCore/Tests/AsterismCoreTests/EntryDetailAndMergeToleranceTests.swift` | Replaced the throw-pinning test with one pinning the tolerant behaviour |+| `Asterism/AsterismTests/EntryDetailModelTests.swift` | Two new model-level tests |+| `specs/library-integrity-tolerance/decision_log.md` | Q39 promoted to full Decision 13 |++## Verification++**Automated:**+- [x] Regression test passes (`make test-core`)+- [x] Full test suite passes (`make test-core`, `make test-quick`)+- [x] `make build-ios` (simulator) confirms the app-side DTO consumers compile+- [x] No linters/formatters configured in this repo (per `CLAUDE.md`); a clean+ `make test-core` with no new compiler warnings is the pre-commit bar++**Manual verification:**+- Not performed against a physical device or the running app — the fix is+ covered by the repository-level and view-model-level regression tests above,+ and `make build-ios` confirms the view compiles against the widened DTO.++## Prevention++**Recommendations to avoid similar bugs:**+- When a tolerance fix demotes a throw to a degraded value in one surface,+ grep for every other reader of the same underlying data before closing the+ task — Recent and Entry detail read the same Site through two independently+ written legality checks, and only one was updated.+- Prefer one shared predicate function over two screens each re-deriving "is+ this record's state legal" — this file's own comments already call out this+ pattern for `duplicatedHostnames(in:)` and `replayCitedPattern`; the same+ discipline should have applied to Site-tuple legality from the start.++## Related++- Transit ticket T-1949+- `specs/library-integrity-tolerance/decision_log.md` Q39 (superseded by+ Decision 13), Q52, Q53+- `specs/library-integrity-tolerance/tasks.md` tasks 15, 17
diff --git a/docs/agent-notes/testing.md b/docs/agent-notes/testing.mdindex 84ff30e..1a749b8 100644--- a/docs/agent-notes/testing.md+++ b/docs/agent-notes/testing.md@@ -139,8 +139,10 @@ of the shapes they exist to produce. Two consequences that are easy to trip over empty diagnosis list and a screen with nothing on it. `.siteTuple` is also the *only* class with a re-teach route: its Recent rows-resolve no Site mode, so they carry no Teach pill (Q38) and Entry detail refuses-outright (Q39). The diagnosis screen is the only way in, which is why+resolve no Site mode, so they carry no Teach pill (Q38), and Entry detail loads+the record with a nil `siteMode`, an inline notice and no Teach action+(Decision 13, T-1949). The diagnosis screen is still the only re-teach route+(Q52), which is why `LibraryDiagnosticsUITests` drives that route end to end. ## AsterismCore suites must run serially (`--no-parallel`)
diff --git a/specs/library-integrity-tolerance/implementation.md b/specs/library-integrity-tolerance/implementation.mdindex 12171e5..fbe5162 100644--- a/specs/library-integrity-tolerance/implementation.md+++ b/specs/library-integrity-tolerance/implementation.md@@ -867,7 +867,6 @@ on both halves — 0.759–0.764 s against 1 s, and 0.314 s against 2 s. | Req 2.1 | Works list and Work detail have no tolerated-state test; tolerance is inherited from `fetchWork` and never exercised (T-1957) | | Req 2.5 | `moveEntry` likewise inherits tolerance and is untested under duplicates (T-1957) | | Req 4.4 | Tested statically as coherent → 0, never as a banner transition (T-1957) |-| Req 2.1 (Entry detail) | Still fails wholesale for a single rule-invalid site, where Recent degrades (Q39, T-1949) | ### Not met
diff --git a/specs/library-integrity-tolerance/decision_log.md b/specs/library-integrity-tolerance/decision_log.mdindex 69d2393..2d2ce06 100644--- a/specs/library-integrity-tolerance/decision_log.md+++ b/specs/library-integrity-tolerance/decision_log.md@@ -55,7 +55,7 @@ | Q49 | 2026-07-26 | A row on a duplicated hostname is `isActionable == false` and carries a new `RecentRowAttention.siteDuplicated` | The count labels a banner reading "N entries need teaching" and filters Recent to exactly those rows, so counting a row whose teaching is refused points the reader at work they cannot do. Q38 already established that attention rows do not inflate `actionableCount`; that reasoning was written for a nil `siteMode`, and a duplicated hostname resolves the winner's mode, so it needed deciding rather than inheriting. The attention mark is what keeps a row whose action was withdrawn from reading as a settled one — Req 2.2's "exactly two unresolvable causes" is about unresolvability, and `.siteRulesInvalid` already exceeded that list (task 15). Attention precedence is `siteMissing` → `siteDuplicated` → `siteRulesInvalid` → `workMissing`, inverting Q24's order: there `.siteTuple` wins because it is actionable, here duplication wins because it is what makes the tuple *un*actionable | | Q50 | 2026-07-26 | A teaching commit invalidates the carried-forward tuple set, not just the quarantine map | `refreshDiagnostics()` unions the scan output with the tuple set held in `diagnostics`, which is a cache of the last full validation. `recordPostCommitDiagnosis` updated only `quarantined`, so a re-teach that cleared a `.siteTuple` left the cache holding it and the very next foreground refresh re-quarantined the hostname — Req 3.1 undone one foreground later, by the mechanism that exists to keep diagnoses fresh. The commit already knows the current answer for the hostname it wrote, so it writes it into `diagnostics` too (`LibraryDiagnostics.recordingTupleDiagnosis`). Not in design.md; it falls out of relaxing the guard in task 21 and would have been a silent hole. The `.duplicate(type: "Site")` reason `quarantineMap()` produces for a duplicated hostname (Q24) is deliberately **not** carried forward: it is not a tuple diagnosis and the scan re-derives its class on its own | | Q51 | 2026-07-26 | The four Req 3.4 write-path guards are **not** among the consequences of breaking the union invariant | design.md and task 24 both say a scan-only republish would "re-enable the four write paths that must refuse". It would not: since Q41 those guards read `diagnostics.diagnoses` for `.duplicateSiteRows`, a class the scan re-derives on every refresh, so they keep refusing either way. The consequences that are real are the two consumers of the quarantine *map*: capture's conservative no-rule path (`+ReparseCapture.swift:284`, `:396`) and the backup export gate (`BackupV4Exporter.swift:41`). `RefreshUnionInvariantTests` asserts all three — the write paths as a weaker regression pin, the other two as the ones that actually bite, verified by mutating the union and watching them fail |-| Q52 | 2026-07-26 | The diagnostics screen is the **only** route to re-teach a genuinely tuple-diagnosed hostname, which makes Req 4.5 load-bearing rather than convenient | Task 23's text asserted a `.siteTuple` hostname "MUST keep offering Teach". In practice it cannot: `validatedRecentSiteMode` returns nil for an illegal tuple (task 15), so the Recent row gets `actionType == .none`, and Entry detail refuses wholesale (Q39). Task 23's pinning tests hold only because they inject a `.siteTuple` diagnosis over a *legal* store, which is the sole way to separate the two states — a genuinely illegal tuple resolves no mode anywhere. So the one clearable class has exactly one repair route, and it runs through the diagnosis screen. `composedTeachingModel(forHostname:)` exists because the row-based `composedTeachingModel(for:)` guards `actionType != .none` and would have refused every hostname the screen routes for |+| Q52 | 2026-07-26 | The diagnostics screen is the **only** route to re-teach a genuinely tuple-diagnosed hostname, which makes Req 4.5 load-bearing rather than convenient | Task 23's text asserted a `.siteTuple` hostname "MUST keep offering Teach". In practice it cannot: `validatedRecentSiteMode` returns nil for an illegal tuple (task 15), so the Recent row gets `actionType == .none`, and Entry detail refused wholesale (Q39; since Decision 13 it degrades to a nil `siteMode` with no action instead). Task 23's pinning tests hold only because they inject a `.siteTuple` diagnosis over a *legal* store, which is the sole way to separate the two states — a genuinely illegal tuple resolves no mode anywhere. So the one clearable class has exactly one repair route, and it runs through the diagnosis screen. `composedTeachingModel(forHostname:)` exists because the row-based `composedTeachingModel(for:)` guards `actionType != .none` and would have refused every hostname the screen routes for | | Q53 | 2026-07-26 | A `.siteTuple` hostname carrying **no Entry** shows a re-teach button that does nothing | The composed teaching surface is entered from an Entry, so `composedTeachingModel(forHostname:)` returns nil and the tap is a no-op. Reachable by teaching a hostname and then deleting all its Entries. Given Q52 this is the *only* route failing for that shape. The minimum fix is to withhold the button when no route exists — consistent with Req 3.4 and with what tasks 15/17/23 did elsewhere. A real fix needs a hostname-only teaching entry point, which no task authorises. Documented in code; left open deliberately | | Q54 | 2026-07-26 | `LibraryProviding.diagnostics` is `{ get async }` | The concrete member is an actor-isolated stored property, and a synchronous witness cannot satisfy a non-async requirement. Verified the witness compiles for both the actor and `MockLibraryProvider` | | Q55 | 2026-07-26 | UI-test fixtures reopen the library after seeding | `.siteTuple` is derived only by the full `validate(graph:)` at open — `refreshDiagnostics()` cannot produce it (Decision 7) — and the fixture is written after the repository has already opened on an empty store. `AppLibraryModel.bootstrap` reopens for fixtures marked `requiresReopenAfterSeeding`. Also recorded in `docs/agent-notes/testing.md` |@@ -831,7 +831,7 @@ Withholding every Site-derived field rather than only suppressing the throw matt **Positive:** - The bug is closed: a reader who taps through from a marked Recent row now sees the same entry, editable, with a plain inline explanation, instead of a blank screen. - Recent and Entry detail cannot drift on "is this Site's tuple legal" again without a compiler-visible change, since both call `validatedRecentSiteMode`.-- The fix reads as a straightforward widening rather than a new subsystem: no new error case, no new enum, one optional field and one gate reused four times.+- The fix reads as a straightforward widening rather than a new subsystem: no new error case, no new enum, one optional field and one `legalSite` gate reused across every Site-derived value. **Negative:** - `validatedRecentSiteMode`'s name still says "Recent," which is no longer accurate now that Entry detail calls it too; renaming was left out of this change to keep the diff to the DTO and the two call sites, and is a candidate for a later cleanup pass.
diff --git a/Asterism/Asterism/ViewModels/EntryDetailModel.swift b/Asterism/Asterism/ViewModels/EntryDetailModel.swiftindex 7e8da80..0be4442 100644--- a/Asterism/Asterism/ViewModels/EntryDetailModel.swift+++ b/Asterism/Asterism/ViewModels/EntryDetailModel.swift@@ -144,13 +144,14 @@ public final class EntryDetailModel { /// Whether this entry's Site retains rules that failed validation (Q39, /// T-1949). This used to fail the whole screen — `.quarantined` was thrown /// out of `entryTeachingDetail` and this Entry looked deleted (`Unavailability- /// .siteRulesInvalid` below still exists to catch that if it ever happens+ /// .siteRulesInvalid` above still exists to catch that if it ever happens /// again). The repository now resolves a nil `siteMode` for exactly this /// condition instead, the same demotion `validatedRecentSiteMode` already /// gave Recent, so the record loads and stays editable; only what the Site /// claims to teach is withheld. `teachingDetail?.siteMode == nil` cannot mean- /// "not loaded yet" here, because `teachingDetail` and `entry` are only ever- /// set together.+ /// "not loaded yet" here, because `load()` sets `teachingDetail` and `entry`+ /// together (a delete clears `entry` alone, and the view then shows the+ /// unavailability screen rather than this flag). public var siteRulesInvalid: Bool { guard let teachingDetail else { return false } return teachingDetail.siteMode == nil
diff --git a/Packages/AsterismCore/Sources/AsterismCore/LibraryRepository+EntryDetail.swift b/Packages/AsterismCore/Sources/AsterismCore/LibraryRepository+EntryDetail.swiftindex 8c3eb34..99fa4ba 100644--- a/Packages/AsterismCore/Sources/AsterismCore/LibraryRepository+EntryDetail.swift+++ b/Packages/AsterismCore/Sources/AsterismCore/LibraryRepository+EntryDetail.swift@@ -124,7 +124,7 @@ extension LibraryRepository { intentionallyUnattached: entrySnap.intentionallyUnattached ) - // Build available actions. Two states offer none, for one reason:+ // Build available actions. Three states offer none, for one reason: // `buildComposedTeachingBasis` refuses both, so an action here would // be the dead end Req 3.4 exists to prevent. //
diff --git a/Asterism/AsterismTests/EntryDetailModelTests.swift b/Asterism/AsterismTests/EntryDetailModelTests.swiftindex aeba50e..8621d36 100644--- a/Asterism/AsterismTests/EntryDetailModelTests.swift+++ b/Asterism/AsterismTests/EntryDetailModelTests.swift@@ -461,6 +461,10 @@ struct EntryDetailModelTests { /// tapped the row it labelled, and the detail screen told them the entry had /// been removed. The entry was there the whole time; its site's rules were /// the thing that was not valid, and the app repairs that on its own.+ /// Since T-1949 the repository no longer throws `.quarantined` for this+ /// condition (it resolves a nil `siteMode`, see+ /// `ruleInvalidSiteLoadsInsteadOfRefusing`); this pins the defensive+ /// fallback mapping in `load()` should the error ever arrive from elsewhere. @Test("A quarantined site is reported as invalid rules, not as a deleted entry") @MainActor func quarantinedSiteIsNotReportedAsDeleted() async { let (model, mock, _) = makeSUT()
"Every on-disk state classifies to the ordered match, and nothing is written" (256 cases) failed on one of three make test-core runs and once in six isolated runs; it passed 3/3 on the main checkout and 4/4 subsequently in this worktree. Nothing in this branch touches bootstrap classification. Worth its own ticket if it recurs.
isReadOnly (torn duplicate group) and siteRulesInvalid are independent, so duplicateReviewSection and siteRulesInvalidSection can render together. Both statements are true; acceptable, but eyeball it if a fixture with both exists.
Model tests cover the flag; the entry-detail-site-rules-invalid-notice accessibility identifier is in place if a UI test is wanted later.