Timeline: TimelineEvent entity + Flyway migration #774
Reference in New Issue
Block a user
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Milestone: Zeitstrahl — Family Timeline
Spec:
docs/superpowers/specs/2026-06-07-family-timeline-design.md§ "Data model"ADR: this issue ships ADR-035 (
docs/architecture/decisions/035-timeline-domain-data-model.md). ADR-033 (tag-name-resolution-tolerates-case-collisions) and ADR-034 (ollama-production-deployment-and-keep-alive) already exist — 035 is next.Context
Curated timeline events need a home. New
timeline/domain package, deliberately separate from the in-flight Lesereisen/Geschichtework (#750–753). This issue is the architectural commitment for the domain: entity + enum + repository + migration + ADR + doc updates. No service/controller/DTO here — those land in issues 3–5. Independently shippable.Scope
TimelineEvententity, mirroring theDocumentdate block so events and letters share one date-handling path. Note the audit footprint intentionally diverges fromDocument(which has neitherversionnor a creator): a curated entity edited by multiple curators warrants optimistic locking + creator tracking. The word "mirror" applies to the date block only — do not assume full parity.Entity fields
idUUID@GeneratedValue(GenerationType.UUID),@Schema(requiredMode = REQUIRED)titleString@Schema(requiredMode = REQUIRED)typeEventTypeenumPERSONAL,HISTORICAL.@Enumerated(STRING),@Column(nullable=false, length=16),@Schema(requiredMode = REQUIRED). Enum value names are a frontend styling hook (personal = family accent, historical = muted world accent) — keep them stable, no mapping layer. Renaming requires a coordinated frontend change (documented in ADR-035).eventDateLocalDate@Schema(requiredMode = REQUIRED)precisionDatePrecisiondate_precision(avoid SQL reserved-ishprecision).@Enumerated(STRING),@Column(name="date_precision", nullable=false, length=16),@Builder.Default DatePrecision.YEAR,@Schema(requiredMode = REQUIRED). Reusesorg.raddatz.familienarchiv.document.DatePrecision— import it, do not duplicate (one rendering path; the enum's javadoc forbids divergence fromtools/import-normalizer/dates.pyper ADR-025).eventDateEndLocalDate(nullable)precision = RANGE(see CHECK below)descriptionString(nullable)@Column(columnDefinition = "TEXT"), matchingDocument.transcription/summarypersonsPerson@Builder.Default new HashSet<>(),@BatchSize(size = 50), explicit@JoinTable(name = "timeline_event_persons", joinColumns = @JoinColumn(name = "timeline_event_id"), inverseJoinColumns = @JoinColumn(name = "person_id"))documentsDocument@Builder.Default new HashSet<>(),@BatchSize(size = 50), explicit@JoinTable(name = "timeline_event_documents", joinColumns = @JoinColumn(name = "timeline_event_id"), inverseJoinColumns = @JoinColumn(name = "document_id"))createdByUUID@Column(name = "created_by", nullable = false)— no FK toapp_users(matchesDocumentAnnotation/OcrJob/InviteTokensidecar pattern; keepstimelinedecoupled fromuser). Server-populated from session principal in issue 3 — never accepted from client input.createdAtLocalDateTime@CreationTimestamp(project idiom — never hand-set).LocalDateTimematchesDocumentAnnotationprecedent.updatedByUUID@Column(name = "updated_by", nullable = false)— same rationale ascreatedByupdatedAtLocalDateTime@UpdateTimestampversionLong@Version— optimistic locking for the multi-curator edit flow (issue 3).Long(object, not primitive) so it isnullbefore first persist.Resolved design decisions
tag-name-resolution-tolerates-case-collisions) and ADR-034 (ollama-production-deployment-and-keep-alive) already exist. 035 is the next available number.createdBy/updatedBy= bareUUID,NOT NULL. Rationale: matches the lighter sidecar-entity precedent (DocumentAnnotation), keepstimeline → userdecoupled, avoids lazy-load surprises in the read-heavy assembly path. Must benullable = falsein entity and DDL — a curated event with no author is an audit gap. Curator display names resolved throughUserServicewhen rendered.createdAt/updatedAttype =LocalDateTime. Rationale: matchesDocumentAnnotationprecedent;Instantwould be a novel divergence requiring new serialization handling across the stack. Both use@CreationTimestamp/@UpdateTimestampwhich work withLocalDateTime.@JoinTableannotations on both ManyToMany fields. Rationale: without explicit@JoinTable(name, joinColumns, inverseJoinColumns), Hibernate's naming strategy may silently diverge from the Flyway DDL's explicit table/column names. Explicit mapping guarantees alignment and future column renames won't cause silent schema mismatches.@Version Long version. Rationale: a curated, multi-curator-editable entity benefits from real optimistic-lock protection; the divergence fromDocument(no version) is deliberate, not accidental.OptimisticLockingFailureExceptionmust be caught and translated toDomainException.conflict(...)in issue 3 — pre-noted in ADR-035.Document's nullable-end open-ended ranges, because a curated event always has a closed, known end when it spans a range (it is authored, not OCR-inferred).precision = UNKNOWNis forbidden for a curated event. Rationale:eventDateis required (an event always has at least a year); only letters fall into the "Ohne Datum" bucket. Enforced by the CHECK below.DatePrecisionstays indocument/. Rationale: moving it is a wider refactor touchingDocument,importing, and the normalizer contract — out of scope and its own future ADR. Thetimeline → documentcompile coupling already has precedent (importing/DocumentImporter). ADR-035 explicitly records this coupling as intentional, citing ADR-025 and the normalizer contract as justification.EventTypestring values are a stable frontend styling contract. Rationale: the Tailwind class map in future Svelte components will hard-codePERSONALandHISTORICALas strings. Renaming either value requires a coordinated frontend change — documented in ADR-035 to prevent silent regressions.Tasks
timeline/TimelineEvent.java— standard Lombok entity conventions (@Entity @Table(name="timeline_events") @Data @NoArgsConstructor @AllArgsConstructor @Builder).timeline/EventType.javaenum (PERSONAL,HISTORICAL).@Schema(requiredMode = REQUIRED)onid,title,type,eventDate,precision.@Builder.Default new HashSet<>()onpersonsanddocuments;@BatchSize(size = 50)on both.@JoinTable(name = "timeline_event_persons", joinColumns = @JoinColumn(name = "timeline_event_id"), inverseJoinColumns = @JoinColumn(name = "person_id"))onpersons; symmetric@JoinTablefordocuments.@Builder.Default DatePrecision.YEARonprecision;@Column(name = "date_precision", nullable = false, length = 16).descriptionas@Column(columnDefinition = "TEXT").@Column(name = "created_by", nullable = false) UUID createdBy,@CreationTimestamp LocalDateTime createdAt,@Column(name = "updated_by", nullable = false) UUID updatedBy,@UpdateTimestamp LocalDateTime updatedAt,@Version Long version.timeline/TimelineEventRepository.java(Spring Data JPA).V72__add_timeline_events.sql(latest on disk isV71__person_delete_on_delete_fk.sql):CREATE TABLE timeline_events(id, title, type, event_date, date_precision, event_date_end nullable, description TEXT nullable, created_by UUID NOT NULL, created_at, updated_by UUID NOT NULL, updated_at, version).CREATE TABLE timeline_event_persons+CREATE TABLE timeline_event_documentsjoin tables with explicit columns(timeline_event_id, person_id)/(timeline_event_id, document_id).IF NOT EXISTS, no destructive statements.ON DELETE CASCADEon both join tables —timeline_event_persons.person_id,timeline_event_persons.timeline_event_id,timeline_event_documents.document_id,timeline_event_documents.timeline_event_id. Deleting a Person/Document drops the join row; the event survives. (Matches V71/ADR-032 hardening; a person delete must never 500.)chk_timeline_event_range:(date_precision = 'RANGE') = (event_date_end IS NOT NULL)— enforces eventDateEnd-iff-RANGE.chk_timeline_event_precision:date_precision <> 'UNKNOWN'— curated events are never undated.timeline_events(event_date),timeline_events(type), and explicit indexes on all four FK columns:idx_timeline_event_persons_person_id,idx_timeline_event_persons_event_id,idx_timeline_event_documents_document_id,idx_timeline_event_documents_event_id.docs/architecture/decisions/035-timeline-domain-data-model.md. Must record: (1) newtimelinedomain + data-model commitment; (2) version/audit/CHECK decisions; (3)timeline → document.DatePrecisioncompile coupling is intentional, citing ADR-025; (4)EventTypevalues are a stable frontend styling contract — renaming requires a coordinated frontend change; (5)OptimisticLockingFailureException → DomainException.conflict(...)translation contract (implemented in issue 3); (6)createdBy/updatedByNOT NULL enforces the audit trail.CLAUDE.mdpackage table — addtimeline/.docs/architecture/c4/l3-backend-timeline.puml.docs/architecture/db/db-orm.pumlanddb-relationships.puml— new table + two join tables.docs/GLOSSARY.md—TimelineEvent,EventType.Forward notes (do NOT implement here — for issues 3/5)
TimelineEventRequestDTO must accept id lists for persons/documents and resolve them viaPersonService/DocumentService— never bind client-suppliedPerson/Documentobjects (mass-assignment guard). The entity must not be exposed as a request body.createdBy/updatedByare never accepted from client input — server-populated from the session principal.ObjectOptimisticLockingFailureExceptionand translate it toDomainException.conflict(...)— pre-noted in ADR-035 so the data-model ADR documents the full lifecycle of the version field.READ_ALLon reads,@RequirePermission(WRITE_ALL)on every write. Regression suite must includecreate/update/delete → 403 for READ_ALL-onlyand401 unauthenticated.Acceptance criteria
@DataJpaTestrunning V72 (the migration test is the boot guarantee; already covered byMigrationIntegrationTestwhich runs all migrations in sequence — no new@SpringBootTestneeded just to verify boot).precisiondefaults toYEARwhen not set via builder.TimelineEventalways has a non-nulleventDate;eventDateEndis non-null iffprecision = RANGE(DB CHECK, both directions).precision = UNKNOWNis rejected at the DB (CHECK).PersonorDocumentremoves the join row and leaves the event intact (FKON DELETE CASCADE).createdByandupdatedByare NOT NULL in the DB — no event can be created without an audit trail.Tests —
@DataJpaTest, Testcontainerspostgres:16-alpine(never H2 — house rule; H2 would hide enum-as-varchar, FK cascade, CHECK, and@Version)Use a
makeEvent(...)factory (sensible defaults: title, eventDate, YEAR precision); each test overrides only what it asserts. One logical assertion per test. TDD red-first order:Tests #1–9:
@DataJpaTest+@AutoConfigureTestDatabase(replace = NONE)+@Import(PostgresContainerConfig.class, FlywayConfig.class)— consistent withMigrationIntegrationTest's existing pattern.persists_and_loads_event_with_required_fields(assert id generated).precision_defaults_to_YEAR_when_not_set(build without precision).persists_event_with_linked_persons(save with Person, flush+clear, reload, assert membership).persists_event_with_linked_documents(same for Document).eventDateEnd_round_trips_null_for_non_range.eventDateEnd_round_trips_value_for_range.description_round_trips_null.description_round_trips_multi_kb_text(prove TEXT has no length cap —@Column(columnDefinition = "TEXT")overrides Hibernate's default VARCHAR(255)).range_invariant_rejects_non_null_end_without_range_precision(precision=YEAR + non-null end → DB rejects).@Transactional(propagation = NOT_SUPPORTED)— constraint violation invalidates the current transaction;NOT_SUPPORTEDprevents poisoning subsequent tests.range_invariant_rejects_range_precision_without_end_date(precision=RANGE + null end → DB rejects).@Transactional(propagation = NOT_SUPPORTED).unknown_precision_is_rejected(CHECK).@Transactional(propagation = NOT_SUPPORTED).Tests #12–13:
@SpringBootTest(webEnvironment = NONE)+@Import(PostgresContainerConfig.class)— requirePersonService/DocumentServicefull Spring context (matchingPersonServiceIntegrationTestpattern).deleting_linked_person_keeps_event_and_drops_join_row(persist event+Person, delete Person viaPersonService, assert event survives + join row gone — proves V72 cascade; permanent V71-class regression guard).deleting_linked_document_keeps_event_and_drops_join_row(symmetric).Coverage gate: 88% branch (
backend/CLAUDE.md, not the generic 80% floor) — the builder defaults and nullable handling are what JaCoCo measures here.🏗️ Markus Keller — Application Architect
Observations
ADR path is wrong. The issue specifies
docs/architecture/decisions/035-timeline-domain-data-model.mdbut that directory does not exist. All 34 existing ADRs live indocs/adr/. The correct path isdocs/adr/035-timeline-domain-data-model.md. This is a concrete merge blocker — the PR will create the file in the wrong place if the implementation follows the issue text verbatim.ADR content scope looks right. The six required ADR sections (domain commitment, version/audit/CHECK decisions,
timeline → document.DatePrecisioncoupling,EventTypecontract, optimistic-lock translation contract,NOT NULLaudit enforcement) are all load-bearing. None is cosmetic.timeline → document.DatePrecisioncompile coupling is deliberate and well-reasoned. The issue correctly cites ADR-025 and the normalizer contract as justification. The javadoc onDatePrecision.javaconfirms the "no translation layer" rule. Importing across domain boundaries is acceptable here; the alternative (duplicating the enum) would silently break import idempotency.DocumentAnnotationprecedent for bare-UUID audit fields is accurately cited. I verified:DocumentAnnotation.createdByis@Column(name = "created_by") UUID createdBy— no FK, nullable in the entity (but the entity has noupdatedBy). The issue correctly escalates toNOT NULLforcreatedBy/updatedByon the curatedTimelineEventbecause an event with no author is an audit gap. The reasoning holds.@Version Long(object type) is the right call. Primitivelongwould never benullbefore first persist, which would confuse Hibernate's new-vs-managed detection. The issue already notes this.CHECK constraint on
(date_precision = 'RANGE') = (event_date_end IS NOT NULL)is strict by design. This is intentionally stricter thanDocument's check (which allows a RANGE with a null end). The issue explains the rationale (curated events always have a known end when spanning a range). The divergence is deliberate and must be recorded in the ADR.Doc-update task list is complete. CLAUDE.md package table, C4 L3 backend diagram, both DB diagrams (main table + two join tables), GLOSSARY.md — all required by the documentation table in my persona. The issue lists them all. No gaps found.
Recommendations
docs/adr/035-timeline-domain-data-model.mdnotdocs/architecture/decisions/035-timeline-domain-data-model.md.Document's open-ended) is intentional divergence — both for future readers and to prevent a "bug fix" that accidentally relaxes it.timeline/package appears in neither CLAUDE.md nor the C4 L3 backend diagram yet; the doc-update tasks capture this correctly. Treat both as merge blockers (per the documentation table), not as nice-to-haves.👨💻 Felix Brandt — Senior Fullstack Developer
Observations
Entity conventions are fully specified.
@Data @NoArgsConstructor @AllArgsConstructor @Builder,@Builder.Default new HashSet<>()on both collections,@Schema(requiredMode = REQUIRED)on the five required fields — all consistent with the codebase pattern (Document,DocumentAnnotation, etc.). No deviations.@BatchSize(size = 50)on both ManyToMany collections is correctly called out. Without it, loading a list ofTimelineEvents with theirpersons/documentscollections would N+1 at the join tables. The existingDocumententity uses@BatchSizeon itsreceiversandtagscollections — verified in the entity source.Explicit
@JoinTableis necessary. Without it, Hibernate's implicit naming strategy may diverge from what Flyway wrote. The issue cites the correct rationale. Verified against existing join tables in the DB diagrams:document_tags,document_receiversall use explicit@JoinTablein their respective entities.@Column(name = "date_precision", nullable = false, length = 16)column rename is critical.precisionis a reserved keyword in SQL and in some JPA/Hibernate versions. The explicitnameattribute prevents silent mapping issues. Good call.@UpdateTimestampandupdatedBywill drift apart.@UpdateTimestampfires on every HibernateUPDATE.updatedByis service-populated (issue 3). If issue 3 forgets to setupdatedBybefore saving, the timestamp advances but the "who" is stale. This is a data-quality risk pre-noted for issue 3, but worth noting here because the entity design creates the coupling. Consider adding a note in the entity Javadoc or in ADR-035.Long version(object, not primitive) is correct.longwould initialize to0on unpersisted instances, confusing Hibernate's dirty-tracking.Longinitializes tonull, which is Hibernate's signal for "unmanaged entity."description @Column(columnDefinition = "TEXT")matchesDocument.transcriptionprecedent. Verified inDocument.java—transcriptionuses@Column(columnDefinition = "TEXT"). Consistent.Recommendations
TimelineEvent.updatedBy: "Populated from session principal inTimelineEventService; must be set before everysave()call —@UpdateTimestamponupdatedAtdoes not set this automatically." This prevents the drift issue without a runtime guard.TimelineEventRepository, consider addingfindByPersonsContaining(Person p)now rather than deferring — it will be needed in issue 5 for the per-person filter. If deferred, add a// TODO: needed in issue 5comment to the empty repository so the implementer knows where to start.makeEvent(...)factory pattern specified in the test section is consistent with project conventions (makeDocument(),makeUser()etc.). Follow it exactly — don't useDocument.builder()directly in test bodies.🔒 Nora "NullX" Steiner — Application Security Engineer
Observations
This issue is data-model-only — no controller, no service, no user-facing input. There are no active injection vectors, CORS surface, or mass-assignment risks in this PR scope. The security exposure lives in issues 3–5. That said, several design choices here have downstream security implications worth documenting now.
createdBy/updatedByas bare UUID withNOT NULLis the right call. The "never accepted from client input" rule (documented in the issue as a Forward Note for issue 3) must be enforced at the service layer, not by the entity. The entity-levelNOT NULLensures no row can be created without an author, but nothing in this PR mechanically prevents a future developer from acceptingcreatedByfrom a request body. The ADR-035 entry documenting this constraint is the defense. Recommend the ADR explicitly name the threat: "acceptingcreatedByfrom client input is a privilege escalation — it allows an attacker to forge authorship."@Version Long versionintroducesObjectOptimisticLockingFailureExceptionsurface. The Forward Note for issue 3 correctly pre-notes that the service must catchObjectOptimisticLockingFailureExceptionand translate it toDomainException.conflict(...). If issue 3 misses this catch, the exception bubbles as a 500 with a Hibernate stack trace — information leakage. Recommend the ADR-035 entry explicitly state that the translation is mandatory and tested in issue 3's regression suite.CHECK constraints as security controls. The
chk_timeline_event_precisionconstraint (date_precision <> 'UNKNOWN') and the RANGE biconditional CHECK are structural integrity guarantees. They can't be bypassed by application bugs or missing service validation. This is the correct approach — "push integrity to Postgres."No FK to
app_usersreduces attack surface. AcreatedByFK would require thetimelinepackage to join or loadapp_usersdata, expanding the access surface. The sidecar-entity pattern (bare UUID resolved throughUserServiceonly at render time) is correctly cited. This is a sound isolation decision.@BatchSize(size = 50)is an N+1 mitigation, not a security control — no concern here.Recommendations
createdBy/updatedByare server-populated only — never bind from request body. Accepting these fields from client input is a CWE-639 (IDOR) vector enabling authorship forgery."POST /api/timeline/eventswith a body containingcreatedBy→ assert that the storedcreatedByis the session principal's UUID, not the supplied value. This test belongs in the controller-level@WebMvcTestsuite.OptimisticLockingFailureException → DomainException.conflict()translation must have a regression test in issue 3. Without it, a concurrent write conflict returns HTTP 500 with Hibernate internals in the response body — an information disclosure (CWE-209).🧪 Sara Holt — QA Engineer & Test Strategist
Observations
Test plan is well-structured and follows the existing
MigrationIntegrationTestpattern exactly. I verified the pattern in the codebase:@DataJpaTest + @AutoConfigureTestDatabase(replace = NONE) + @Import({PostgresContainerConfig.class, FlywayConfig.class})— this is exactly whatMigrationIntegrationTestuses. ThePersonServiceIntegrationTestpattern (@SpringBootTest(webEnvironment = NONE)) is also verified. Tests #12–13 are correctly using the heavier context because they needPersonService/DocumentService.@Transactional(propagation = NOT_SUPPORTED)on constraint-violation tests is correct. Verified inMigrationIntegrationTest:v30_partialUniqueIndex_preventsTwoRunningTrainingRunsandv64_rejectsDuplicateGroupPermissionboth useNOT_SUPPORTED. The issue correctly applies this pattern to tests #9–11 (CHECK constraint violations).88% branch coverage gate is the backend's actual gate. Confirmed in
backend/CLAUDE.md: "Coverage gate: 88% branch coverage (JaCoCo)". The issue correctly quotes this — not the generic 80% from CLAUDE.md. Good spec precision.makeEvent(...)factory with sensible defaults is the right pattern. Consistent withmakeDocument(),makeUser()helpers found in the existing test suite. One-liner call overriding only what the test asserts.One logical assertion per test is required — the issue states this explicitly. Each of the 13 tests asserts exactly one behavior. No compound assertions.
Tests #12–13 cascade tests need explicit join-row verification. The issue says "assert event survives + join row gone." The assertion for "join row gone" must query the join table directly via
JdbcTemplate(likeMigrationIntegrationTestdoes for constraint verification) or via the entity's collection after flush+clear. UsingJPQL SELECTagainsttimeline_event_personsviaJdbcTemplateis the cleanest approach — consistent with the existing test pattern.The
description_round_trips_multi_kb_texttest (test #8) is a quality call. Verifying that@Column(columnDefinition = "TEXT")actually bypasses Hibernate's default VARCHAR(255) is a real regression guard. V69 migration tests do similar things formeta_date_raw. Good pattern to follow.Missing: a test that
versionstarts asnullon an unpersisted entity and increments after first save. The issue documentsLong versionisnullbefore first persist — this is the whole reason for object vs primitive. A testversion_is_null_before_persist_and_increments_after_savewould permanently document this behavior and catch a refactor that switches to primitivelong.Recommendations
version_is_null_before_persist_and_increments_after_save— build an event (don't save), assertversionis null; save + flush, assertversionis 0 (or ≥ 0). This is a@DataJpaTestone-liner that locks the@Version Longbehavior the issue explains.JdbcTemplate.queryForObject("SELECT COUNT(*) FROM timeline_event_persons WHERE timeline_event_id = ?", Integer.class, eventId)and assert== 0after the cascade delete. The entity'spersonscollection alone won't prove the DB row is gone without hitting the DB.MigrationIntegrationTest: explicit DELETE in a finally block. For tests using rawJdbcTemplate, this matters; for@DataJpaTestwith@Transactional, the rollback handles it — butNOT_SUPPORTEDopts out of the transaction, so explicit cleanup is needed.🚀 Tobias Wendt — DevOps & Platform Engineer
Observations
This is a pure backend data-model issue — no infrastructure changes, no Docker Compose changes, no new services. From an infra/CI perspective, V72 is additive DDL (no
IF NOT EXISTS, no destructive statements) which is the correct approach for Flyway on a live database.Flyway migration V72 is forward-only additive. The issue explicitly calls this out: "Forward-only, additive DDL only — no
IF NOT EXISTS, no destructive statements." This is correct for a Flyway migration that will run against a production database. ADROPorALTER COLUMNin this migration would be a production incident waiting to happen.The Testcontainers
postgres:16-alpinepin is consistent with CI. The project already pins topostgres:16-alpineeverywhere (verified inMigrationIntegrationTest,PostgresContainerConfig). V72 will be exercised by the existingMigrationIntegrationTeston every CI run — the migration boot guarantee is already in place.Index coverage is comprehensive and called out explicitly. The issue lists 7 explicit indexes:
timeline_events(event_date),timeline_events(type), and FK column indexes on all four join-table FK columns. The issue explicitly references the "V62 FK-index retrofit debt" as the reason to add these now. I verified: V62 was indeed a retrofit migration that added indexes missed at table creation. Correct lesson applied.The ADR path issue (flagged by Markus) would not break CI — it would just create the file in the wrong directory. The Flyway migration itself has no dependency on the ADR path.
No new Docker service, no new compose configuration. The
timelinedomain is pure PostgreSQL tables — no new infrastructure dependencies whatsoever.Recommendations
./mvnw test -Dtest=MigrationIntegrationTestlocally before pushing to verify the migration runs clean against a fresh Postgres container. This is the cheapest pre-push verification available.TimelineEvent.javaandEventType.javaclasses. JaCoCo will count the@Builder.Defaultfield initializers and@Enumeratedpaths as branches. The 13 tests in the issue are designed to hit all of them — no concern there.📋 Elicit — Requirements Engineer
Observations
Scope boundary is clean and well-enforced. The issue declares "No service/controller/DTO here — those land in issues 3–5" and the Forward Notes section explicitly defers
TimelineEventRequest,createdBypopulation, andOptimisticLockingFailureExceptiontranslation. The acceptance criteria are scoped to what this PR actually delivers: entity persistence, migration, cascade behavior, and CHECK constraints.Acceptance criteria are measurable and testable. Each criterion maps to at least one numbered test case. No criterion is vague ("fast," "reliable," "clean"). The biconditional RANGE/end-date constraint is specified precisely enough that two separate test cases (#9 and #10) cover both directions.
The
precision = UNKNOWNforbidden rule has a clear rationale. "An event always has at least a year; only letters fall into the 'Ohne Datum' bucket." This is domain knowledge encoded as a DB constraint. No ambiguity.Forward notes are correctly deferred. The mass-assignment guard, optimistic-lock translation, and permission gating are all documented as "do NOT implement here — for issues 3/5." This is good scope hygiene. The risk is that implementers of issues 3–5 miss these notes — consider making them explicit tasks in those issue bodies when they're created.
One gap in the acceptance criteria: there is no AC for "the migration is reversible or safe to roll back." The issue says "forward-only, additive DDL only" but the acceptance criteria don't test rollback behavior. This is probably intentional (Flyway migrations are forward-only in this project) but could be made explicit: "Non-goal: migration rollback. V72 is forward-only; rollback requires manual DDL."
The issue number sequencing claim needs verification. The issue states "ADR-033 and ADR-034 already exist — 035 is next." Verified:
docs/adr/033-tag-name-resolution-tolerates-case-collisions.mdanddocs/adr/034-ollama-production-deployment-and-keep-alive.mdboth exist. ADR-035 is correctly the next number.EventTypeas a frontend styling contract is a non-functional requirement that is functionally enforced. The issue correctly documents thatPERSONALandHISTORICALare hardcoded into the future Svelte class map. Renaming requires a coordinated frontend change. This is an NFR with implementation consequences — the ADR is the right place to document it.Recommendations
DROP TABLEDDL." This prevents confusion during incident response.🎨 Leonie Voss — UX Designer & Accessibility Strategist
Observations
This is a pure backend data-model issue — no UI, no frontend component, no i18n keys in scope. My perspective is largely "no concerns" for this PR, but a few design decisions here have downstream UX consequences worth flagging now.
EventTypevalues (PERSONAL,HISTORICAL) are a styling contract. The issue documents that future Svelte components will hard-code these strings in a Tailwind class map (e.g.PERSONAL → family accent color, HISTORICAL → muted world accent). From a UX perspective, this is the right abstraction — the two types have meaningfully different visual identities (one foregrounds the family, one contextualizes them in the world). The constraint that renaming these values requires a coordinated frontend change is sound.precisiondefaulting toYEARis the correct UX default. Most curated events will be year-precision (wars, moves, marriages by year). A default ofDAYwould produce spurious "28. Juli" labels for events where only the year is known.YEARrenders as "1923" — honest and unambiguous.descriptionas nullable TEXT is right for MVP. Not every curated event needs a narrative — a birth or a war is self-explaining. Optional freetext with no length cap leaves room for rich detail without forcing it.The
eventDaterepresentation for year-only events (e.g.1920-01-01withprecision=YEAR) is UX-critical. The spec confirms this pattern ("vague year →1920-01-01"). Future rendering components must useDatePrecisionto decide what to show — never display "1. Januar 1920" when precision isYEAR. This rendering rule lives in issue 6 (dateLabel.ts), not here, but the data model enables it correctly."Ohne Datum" bucket for
UNKNOWNprecision is correctly forbidden for curated events at the DB level. From a UX standpoint, this is excellent: curated events are author-controlled, so "we don't know when this happened" is never acceptable for a hand-written event. TheUNKNOWNbucket exists only for OCR-inferred letter dates. The DB CHECK enforcement means no UI validation bug can accidentally create an undated curated event.Recommendations
dateLabel.ts) is implemented, note that the1920-01-01representation must never leak to the UI forYEAR-precision events — the helper must strip the month/day entirely. The spec's rendering table is clear on this, but it's easy to forget when writing the formatter.YEAR) and a contextual hint explaining what each precision level means (e.g. "Nur das Jahr ist bekannt"). This is not blocking for this PR — flagging it now saves a UX review round on issue 9.🗳️ Decision Queue — Action Required
2 decisions need your input before implementation starts.
Architecture
docs/architecture/decisions/035-timeline-domain-data-model.mdbut that directory does not exist in the repo. All 34 existing ADRs live indocs/adr/. The correct path isdocs/adr/035-timeline-domain-data-model.md. If the implementation follows the issue text verbatim, the ADR lands in the wrong place and the documentation table in CLAUDE.md becomes out of sync. Fix the path in the issue body before starting. (Raised by: Markus)Testing
version_is_null_before_persist_and_increments_after_savetest — The issue specifies@Version Long version(object type) precisely becausenullsignals "unmanaged entity" to Hibernate. There is no test in the 13-test plan that verifies this behavior. This is a@DataJpaTestone-liner that permanently locks theLongvslongdecision and would catch a future refactor that switches to primitive. Decide: add it to the task list now, or accept the gap and note it as a follow-up. (Raised by: Sara)All other findings are concrete recommendations with no blocking decision required.