Recertify DATA_MODEL.md against the live EF model (#1470) - #1629
Merged
Conversation
Reconciled every entity block against TaskdeckDbContext, the domain entities, and TaskdeckDbContextModelSnapshot.cs. Ten mapped DbSets had no block at all: ProposalRevision, ProposalOutcome, ProposalFeedback, ProposalProvenance, ProvenanceField, ProvenanceEvidenceLink, ConnectorCredential, DailySnapshot, TomorrowNote, and McpToolHash. All 51 mapped entities are now documented, and the ERD and Relationship Summary cover the new edges. Column-level drift corrected: - LlmUsageRecord was missing Status and ExpiresAt, the two columns the quota reservation flow turns on. - Card.Description claimed max 2000; the column is 4000 and 2000 is the domain rule. Both are now stated, matching how Board.Description already reads. - The domain-only note claimed two unmapped entities; there are eight. - "UpdatedAt is a concurrency token on key entities" now names the exact seven that configure it, since it enforces nothing on the rest. Replaced the hand-maintained "855 lines, 37 entities" figure in its three historical delivery ledgers with a pointer to the living document, keeping the as-delivered date and PR. DATA_MODEL.md instead carries a self-checkable coverage invariant with the command to re-derive it. Verified: node scripts/check-docs-governance.mjs passes. Closes #1470 Signed-off-by: Chris0Jeky <jeky.tck@gmail.com>
Owner
Author
|
Worktree teardown note: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7b5666525e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
All seven were checked against the code before being applied; all seven confirmed, none declined. - STATUS.md:84 and IMPLEMENTATION_MASTERPLAN.md:84 still tracked #1470 as open ("DATA_MODEL omits ApprovedRevisionId/DeferredUntil, 855 lines/37 entities is stale") while this same PR closes it. Both residual clauses now record the recertification instead. Masterplan Last Updated bumped. - Added the missing User -> CardCommentMention FK edge to the ERD and the Relationship Summary. CardCommentMentionConfiguration.cs:39-42 and the model snapshot both define MentionedUserId as a cascading FK to User, so the summary's "lists every FK in the model snapshot" claim was false. An audit of all 49 snapshot FK edges found this to be the only gap. Delete behavior also added to the entity's own field table. - The coverage self-check was one-directional: a lone `comm -23` cannot see an unexpected extra heading or a duplicate one, so it could not prove the invariant it was printed under. Now three checks, and the expected output of each is stated. - Rewrote that recipe with `rg` per AGENTS.md:28 (native ripgrep for repo search), replacing two `grep -o` calls. - Dropped "atomic" from the quota-reservation description. STATUS records the check-then-insert as an open over-admission race deferred to #1435, and all four LlmQuotaReservationConcurrencyTests carry Skip = "Reservation atomicity TOCTOU deferred to #1435". Replaced with an explicit "Not atomic" note that separates settlement (closed by #1427) from admission (open). - Documented ConnectorCredential.AuthMethod as string-backed: ConnectorCredentialConfiguration.cs:21-24 applies HasConversion<string>() with HasMaxLength(50) and the snapshot stores TEXT. Added it to the enum-storage exceptions, which are now enumerated exhaustively (three HasConversion<string>() calls exist in Configurations/, no more). - Corrected "last-specific-wins" to first-specific-wins. ProposalFeedbackService.cs guards the rewrite with `existing.Reason == Unspecified`, so Unspecified -> Irrelevant -> TooRisky ends at Irrelevant. Documented as a one-time upgrade out of Unspecified with that exact sequence as a worked table. Verified: node scripts/check-docs-governance.mjs green; the newly documented three-check recipe was executed against the edited document and produced exactly the stated output (51 mapped entities, 53 headings, checks 1 and 3 empty, check 2 exactly AbuseActor and AbuseEvent). Refs #1470 Signed-off-by: Chris0Jeky <jeky.tck@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6bdc12332e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
All four confirmed against code before applying: - ProposalProvenance is at-most-one, not "Exactly one". AutomationProposalService writes it only when the optional IProposalProvenanceRepository was injected, the repository returns a nullable, and 20260425232031_AddProposalProvenanceForeignKey added the FK without a backfill. Prose now matches the ERD and Relationship Summary. - Ground-truth glob broadened to Domain/**: McpToolHash is at Domain/Agents/McpToolHash.cs, the only mapped entity outside Domain/Entities (measured across all 51 DbSet names). - User -> OAuthAuthCode added as a logical edge (ERD, legend, Relationship Summary, persistence note, field row). UserId is required in both flows and the link-flow exchange compares it to the caller; there is simply no DB FK. - TomorrowNote documented as a same-day key: Paper Today saves and fetches under the current dossier date, so no X -> X+1 shift exists. The "tomorrow" framing is recorded as intent that the UI copy and the controller XML doc still assert. Signed-off-by: Chris0Jeky <jeky.tck@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 50b8e5496f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…#1629 round 3) Codex round-3 P2s: qualify prose that claimed guarantees the code does not enforce. - ApprovedRevisionId: add a distinct logical (no-FK) edge row for the approved revision pin, opposite direction to the ProposalRevision.ProposalId cascade FK; MaterializeEffectiveProposalAsync reads it to pick what Apply executes. - UpdatedAt: qualify "every other entity" to "every other entity that maps UpdatedAt"; name the ArtefactBlob/CardLabel/RegistrationBootstrap exceptions. - OAuthAuthCode: narrow the caller-identity check to the authenticated link flow; the anonymous GitHub/OIDC login exchanges run no caller check. - McpToolHash: describe the stored/intended re-approval gate; no MCP execution path calls RecordToolDefinitionAsync/IsToolApprovedAsync (runtime tracked #1154). - Card.BlockReason: 500 is a model width only; Card.Block rejects blank only. - ProposalOutcome: content-free is a caller convention, not an enforced guarantee (SourceType/RiskLevel/ModelId are free-form strings, nonblank+length only). - LlmUsageRecord: Provider(100)/Model(200) are model widths, not runtime-enforced. Signed-off-by: Chris0Jeky <jeky.tck@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6ea397d227
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
This was referenced Aug 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
docs/architecture/DATA_MODEL.mdhad drifted from the live EF model, and the snapshot figure its mirrors advertised for it was stale.Two things had changed since #1470 was filed. The two columns named in the issue title —
AutomationProposal.ApprovedRevisionIdandDeferredUntil— were already added by a later pass (the file'sLast Verified: 2026-08-01line). What remained was the recertification the issue actually scoped: the document carried its own boundary note admitting it was "not yet a complete inventory of all 51 liveDbSetmappings" and named #1470 as the owner. It was documenting 41 of 51 mapped entities.What was reconciled
Ground truth:
backend/src/Taskdeck.Infrastructure/Persistence/TaskdeckDbContext.cs(theDbSetroster),backend/src/Taskdeck.Domain/Entities/*.cs(validation rules), andbackend/src/Taskdeck.Infrastructure/Migrations/TaskdeckDbContextModelSnapshot.cs(persisted columns, widths, indexes, delete behavior).Entities added — 10 mapped
DbSets had no block at allProposalRevisionApprovedRevisionIdpins. Undocumented while the column pointing at it was documented.ProposalOutcomeSourceType/RiskLevelare stored as strings, not the enums of the same name onAutomationProposal.ProposalFeedbackProposalProvenanceProvenanceFieldExtractiveQuoteis required whenKind = Extractiveand must be null otherwise.ProvenanceEvidenceLinkEvidenceLink.ConnectorCredentialDailySnapshotTomorrowNoteMcpToolHashTaskdeck.Domain.Agents, not.Entities.The Mermaid ERD and the Relationship Summary were extended with the corresponding edges (proposal-lifecycle cascades, the two
ConnectorCredentialFKs, and the logical per-user links).Column-level drift corrected in existing blocks
LlmUsageRecordwas missing two live columns.Status(Reserved/Committed) andExpiresAtare what the atomic quota-reservation flow turns on — a reservation only counts toward quota whileExpiresAt > now. Its index list was absent too.Card.Descriptionsaid "Max 2000 chars". The column is 4000; 2000 is the domain rule. Now states both, matching the shapeBoard.Descriptionalready used ("Max 500 chars (DB); domain allows 1000").AbuseActorandAbuseEventare the unmappedEntitysubclasses. There are eight: alsoEvidenceLink,IntentCandidate,IntentEnvelopeV1,SourceBlock,SourceSpan,TaskdeckProposalBatch.UpdatedAtused as optimistic concurrency token on key entities" was unfalsifiable. It now names the exact seven that configureIsConcurrencyToken; on every other entityUpdatedAtenforces nothing — a materially different fact for anyone reasoning about lost updates.ApiKeygained the real DB constraints (KeyHashmax 64 unique;KeyPrefix_persists as columnKeyPrefix, max 10) — the property/column rename was invisible before.Card.BlockReason,ConnectorEvent,AutomationProposalOperation,AgentRunEventgained missing width/index rows.OAuthAuthCode,RegistrationBootstrap, andRegistrationInvitecorrectly have no line — previously their absence looked like an omission.Count-figure decision
Dropped from all three mirrors; not reintroduced as a number anywhere.
Per the issue's own recommendation, the three dated ledger entries keep their as-delivered date and PR (
#875/#917, 2026-04-22) but their shape claim is replaced by a pointer to the living document —docs/STATUS.md:407,docs/AUDIT.md:258,docs/IMPLEMENTATION_MASTERPLAN.md:1690. (The masterplan line has drifted from the:1623recorded in the issue;AUDIT.md:258is still exact.)Instead of a refreshed count,
DATA_MODEL.mdnow carries a self-checkable invariant: everyDbSetonTaskdeckDbContexthas a###block, and the only non-DbSetblocks are the domain-only entities named under the ERD — with thecommone-liner that prints nothing when the doc is current. A count is a fact that rots silently; an invariant plus its check command does not.Verification
node scripts/check-docs-governance.mjs→Docs governance check passed.(exit 0)Coverage invariant verified mechanically, not by eye — extracting both sets and diffing them:
Spot-checks, doc line vs. source line:
LlmUsageRecord.Status/.ExpiresAt—DATA_MODEL.md:787-788⟵LlmUsageRecord.cs:24(Status, defaultCommitted) and:31(ExpiresAt), persisted atTaskdeckDbContextModelSnapshot.cs:1446and:1427; index(Status, ExpiresAt)at snapshot:1464.Card.Description4000 vs 2000 —DATA_MODEL.md:194⟵ columnHasMaxLength(4000)at snapshot:681, domain throw atCard.cs:80-81("cannot exceed 2000 characters").ProvenanceField.ExtractiveQuoteconditional requirement —DATA_MODEL.md:441⟵ProvenanceField.cs:63-68(required forExtractive, rejected otherwise, max 2000); column at snapshot:2024.ApiKey.KeyPrefix_→ columnKeyPrefix—DATA_MODEL.md:570⟵HasColumnName("KeyPrefix")withHasMaxLength(10)at snapshot:237-241; property atApiKey.cs:25.DATA_MODEL.mdPersistence Notes ⟵ everyIsConcurrencyToken()in the snapshot::513(AutomationProposal),:1144(DailySnapshot),:1812(ProposalFeedback),:1875(ProposalOutcome),:1916(ProposalProvenance),:2003(ProvenanceEvidenceLink),:2040(ProvenanceField).Indexes and delete behaviors for every entity touched were taken from the snapshot's
HasIndex/OnDeletecalls, including the twoConnectorCredentialcascades (snapshot:2538-2551) and the(ConnectorId, UserId)unique index (:1092).NOT verified: no build or test run — this is a documentation-only change with no executable surface. The doc is reconciled against the EF model snapshot rather than a live
.dbschema; the snapshot is the generated source of truth for migrations, so a hand-edited database could still differ.Residual risk
docs/STATUS.md:84anddocs/IMPLEMENTATION_MASTERPLAN.md:84describe DATA_MODEL.md omits ApprovedRevisionId and DeferredUntil, and its snapshot figure is stale #1470 as an open residual and restate the stale figure. Both are out of this PR's ownership (a coordinator pass owns that content) and were deliberately left alone; they go stale when this merges and should be swept with the other residual-list updates.Closes #1470