Skip to main content

App History Design Review

Scope​

This report reevaluates finding #3 from v2-discrepancy-report.md against the rules in design-guidelines.md, using the current app_history table and the existing V1/frontend behavior as the concrete baseline.

It answers two questions:

  1. Should app approval events, notes, and audit trail exist as a first-class V2 resource?
  2. Is the current app_history table a sound backing model for that resource?

Inputs Reviewed​

Executive Summary​

Finding #3 remains valid.

The missing V2 app history contract is not just a docs gap. It is a resource-model gap:

  • the frontend currently depends on app-scoped history for approval changes and notes,
  • the backend already persists these events in a dedicated app_history table,
  • the design guidelines favor a separate child collection for unbounded, time-ordered, non-static data.

Conclusion: app history should be a first-class V2 child resource under an app. The current table is a workable foundation, but the API contract should normalize its shape rather than exposing the table too literally.

Findings​

1) App history should be a separate endpoint, not embedded in App​

This follows the embedding rule directly.

App -> History Events fails all three embed conditions from the guidelines:

ConditionResultWhy
BoundedNoHistory is open-ended over time
Always needed for displayNoApp list/detail can render without full timeline history
Static reference dataNoEvents are mutable business activity, not static metadata

Decision: expose history as its own endpoint, not as an embedded field on App.

This is fully aligned with the guideline:

Embed small, stable, always-needed reference objects. Everything else gets its own endpoint.

2) The current V1 route shape is directionally correct​

The existing V1 routes are:

  • GET /clients/{client_id}/apps/{app_id}/history
  • POST /clients/{client_id}/apps/{app_id}/history
  • POST /clients/{client_id}/apps/{app_id}/history/notes

That shape largely fits the guidelines:

  • history is modeled as a child collection under a specific app,
  • reads use GET,
  • writes use POST,
  • the list endpoint is cursor-paginated and returns { data, total_count, next_cursor }.

The only design weakness is that POST /history mixes two concerns:

  • changing the app approval state,
  • creating an audit event.

From a resource-design perspective, the approval state change belongs to the app or approvals domain, while the history entry is the resulting audit record. V1 collapses both into one endpoint for UI convenience.

Recommendation: keep history as the read model, but make the write model explicit in V2:

  • approval writes should happen through app approval endpoints,
  • note creation can remain a history child-resource write,
  • approval endpoints should create history records as a side effect.

3) The current table design is a valid audit-log foundation, but not a clean API shape​

Current table:

ColumnAssessment
idGood. Stable event identifier. Matches guideline use of id.
app_idGood in storage. Should not be required in the DTO when the app is already in the URL path.
client_idGood in storage for tenancy and indexing. Should be omitted from V2 DTOs because the guidelines say path-scoped tenant IDs stay out of payloads.
event_typeGood. Required event discriminator is necessary.
actor_user_idGood. Nullable is appropriate for system-generated events.
created_atGood. Matches audit timestamp guidance.
commentGood for note-like or annotated approval events.
messageAcceptable but weakly defined. Useful for display, but should be treated as presentation text, not canonical business state.
statusWeak. It encodes approval state as free-text (approved, not_approved) instead of a typed domain field.
metadataUseful escape hatch, but under-specified. Without event-specific shape rules it can become a junk drawer.

The table is good enough as persistence because audit/event tables often need some flexibility. It is not a good direct DTO because several columns are storage-oriented rather than consumer-oriented.

4) The table already proves the resource exists in the domain​

This is not a speculative future feature:

  • ingestion writes app_created events,
  • approval updates write approval_status_changed events,
  • notes are inserted as note_added,
  • the frontend renders the timeline as a user-facing feature today.

So the correct V2 decision is not "do we need history?" but "what is the stable V2 contract for history?"

5) The current pagination contract needs a stable tie-breaker​

The implementation orders history by created_at DESC and uses a cursor derived from the last event id, but the SQL page boundary is still based only on created_at.

That is not a stable cursor when multiple events share the same timestamp:

  • two rows can have the same created_at,
  • page 1 can end on one of those rows,
  • page 2 filters on created_at < boundary_created_at,
  • sibling rows with the same timestamp can be skipped.

This is an API-design issue as much as an implementation issue because the guidelines require cursor-based pagination for collections.

Recommendation:

  • sort by (created_at DESC, id DESC),
  • encode both values in the cursor,
  • page with tuple comparison semantics rather than timestamp alone.

6) Actor identity should be modeled as an embedded actor ref, not split scalar fields​

The current V1 payload uses:

  • actor_user_id
  • actor_display_name

That is workable, but it is not the cleanest V2 shape.

The repo-wide DTO pattern favors embedding a small reference object when another entity appears inside a non-primary resource. However, this should not reuse UserRef.

Why UserRef is wrong here:

  • UserRef is the end-user users domain DTO,
  • app-history actors currently come from partner_users,
  • UserRef includes end-user fields such as username, user_key, and last_activity that do not belong on an audit event actor.

Recommendation:

  • replace actor_user_id + actor_display_name with actor: ActorRef | null,
  • define ActorRef as an audit-oriented embedded DTO, not as UserRef,
  • keep it intentionally small.

Recommended shape:

{
  "actor": {
    "id": "1b8fbc0f-f234-4da7-9cb2-5ae10ef63b8e",
    "display_name": "Jane Smith"
  }
}

If the domain may later include multiple actor sources, this can evolve safely to:

{
  "actor": {
    "type": "partner_user",
    "id": "1b8fbc0f-f234-4da7-9cb2-5ae10ef63b8e",
    "display_name": "Jane Smith"
  }
}

This keeps the event DTO aligned with the existing embedded-ref pattern without binding it to the wrong resource domain.

7) Actor display name is not audit-stable today​

Even if the event DTO moves to actor: ActorRef | null, the audit-stability problem remains. The current table does not store actor display name, and the read path joins partner_users at query time.

That means historical output can change when:

  • a user is renamed,
  • a user is deleted or anonymized,
  • display-name formatting rules change.

For a simple activity feed this may be acceptable. For an audit trail it is a weakness because the event does not fully preserve what was known at write time.

Recommendation:

  • if "audit trail" is the intended contract, snapshot actor.display_name into the event record at write time,
  • if that is too strong a requirement, document that the returned actor label is a presentational join, not immutable audit data.

8) The main guideline mismatch is omission, not structure​

Against design-guidelines.md, the biggest violation is that V2 omits a needed separate resource even though the underlying data is:

  • unbounded,
  • time-ordered,
  • user-visible,
  • already backed by a dedicated table,
  • already required by the frontend.

The design guidelines do not require every table to get an endpoint. But once a workflow depends on an unbounded child collection, the guidelines point toward a dedicated endpoint. That is the missing piece.

Table-Level Evaluation​

What is strong​

  • Dedicated audit table instead of overloading apps.updated_at
  • Correct parent linkage through app_id
  • Tenant scoping through client_id
  • Proper event timestamp via created_at
  • Support for human and system actors through nullable actor_user_id
  • Index on (client_id, app_id, created_at DESC) matches the primary read pattern

What is weak​

  • event_type is unconstrained varchar
  • status is unconstrained varchar
  • message overlaps with derived UI copy and can drift in wording
  • metadata has no documented per-event contract
  • actor identity is currently exposed as split scalar fields instead of a small embedded ref
  • actor display name is not snapshotted in the event record
  • pagination currently depends on created_at ordering without a stable tie-breaker
  • no updated_at, which is fine for immutable audit events, but that immutability should be explicit in docs

What should stay out of the public DTO​

Per the guidelines, these DB fields should not be part of the default V2 event payload:

  • client_id
  • app_id

They are already supplied by the URL path context.

Proposed V2 Resource Model​

Read model:

  • GET /clients/{client_id}/apps/{app_id}/history

Write models:

  • POST /clients/{client_id}/apps/{app_id}/history/notes
  • POST /clients/{client_id}/apps/approvals
  • POST /clients/{client_id}/apps/approvals/remove

Behavior:

  • approval endpoints update app approval state and append history events,
  • note endpoint appends a note event only,
  • history list returns the full audit trail in reverse chronological order.

This keeps business commands separate from the audit timeline while preserving the current UX.

Recommended V2 event DTO:

{
  "id": "9f4f8c52-3d85-4d0f-bec3-d95e890d1d24",
  "event_type": "approval_status_changed",
  "actor": {
    "id": "1b8fbc0f-f234-4da7-9cb2-5ae10ef63b8e",
    "display_name": "Jane Smith"
  },
  "message": "approved this app",
  "comment": null,
  "metadata": {
    "is_approved": true
  },
  "created_at": "2026-03-06T19:42:11.000Z"
}

Notes:

  • omit client_id and app_id from the DTO,
  • keep id and created_at,
  • keep event_type as the stable discriminator,
  • use actor: ActorRef | null rather than split actor scalar fields,
  • keep comment, but treat it as primarily note-event content rather than the canonical representation of approval changes,
  • keep message only if the API intends to own timeline copy,
  • prefer event-specific metadata over exposing a generic status string.
  • App.is_approved and AppHistoryEvent.metadata.is_approved should remain distinct:
    • App.is_approved is the app's current state
    • AppHistoryEvent.metadata.is_approved is the result of that specific approval event

Suggested reusable embedded DTO:

{
  "id": "1b8fbc0f-f234-4da7-9cb2-5ae10ef63b8e",
  "display_name": "Jane Smith"
}
FieldTypeNullableDescription
idstring (uuid)NoStable unique identifier for the actor record
display_namestringNoHuman-readable name to render in timeline UI

Notes:

  • this is intentionally not UserRef,
  • it is an audit-oriented identity projection, not a full actor resource,
  • if system-generated events need to be represented without a separate sentinel string, use actor: null,
  • if multiple actor domains are expected later, extend this to include type.

Future-compatible variant:

{
  "type": "partner_user",
  "id": "1b8fbc0f-f234-4da7-9cb2-5ae10ef63b8e",
  "display_name": "Jane Smith"
}

I would not add username, user_key, last_activity, or other UserRef fields here. They are not needed to render app-history events and would couple the audit DTO to the wrong domain model.

Event type recommendations​

At minimum, preserve these event types:

  • app_created
  • approval_status_changed
  • note_added

app_edited should only remain if there is a real producer and a documented payload shape for it.

Metadata recommendations​

If metadata is exposed, document it per event_type.

Example:

  • approval_status_changed
    • is_approved: boolean
  • app_created
    • no metadata required
  • note_added
    • no metadata required unless future note-editing semantics are added

Avoid making consumers infer business meaning from message text.

Should status exist?​

Not in the V2 API contract.

Reasons:

  • it duplicates information already implied by event_type plus metadata,
  • it is stored as free text rather than a strongly typed field,
  • it is approval-specific and does not generalize well across all event types,
  • it leaks storage normalization into the public contract.

Recommendation: keep status internal for back-compat if needed, but do not document it as part of the V2 DTO.

Design-Guideline Compliance​

Aligns well​

  • Separate endpoint for unbounded child collection
  • Cursor-paginated list response
  • id as the resource identifier
  • created_at audit timestamp
  • Omission of path-scoped tenant IDs from DTOs

Needs explicit documentation​

  • history is append-only and immutable,
  • event ordering is newest-first,
  • next_cursor pagination contract,
  • event-specific meaning of metadata,
  • which writes create history side effects.

Approve app history as in-scope for V2 and document it as a dedicated child collection.

Use the current table as the persistence model, with these contract decisions:

  1. Expose GET /clients/{client_id}/apps/{app_id}/history in V2.
  2. Expose POST /clients/{client_id}/apps/{app_id}/history/notes for notes.
  3. Keep approval writes on approval endpoints, not on POST /history.
  4. Do not expose client_id, app_id, or raw status in the public event DTO.
  5. Define a stable event_type enum and event-specific metadata shapes.
  6. Make history pagination deterministic with (created_at, id) cursor ordering.
  7. Expose actor as ActorRef | null, not actor_user_id plus actor_display_name.
  8. Decide whether actor display name is immutable audit data and, if yes, persist a snapshot.

Follow-on docs to create​

This report supports adding the following docs next:

  • docs/api/apps/history.md
  • docs/api/apps/history-notes.md
  • update docs/api/apps/index.md to include history endpoints
  • update docs/api/v2-discrepancy-report.md finding #3 from "missing/undecided" to "accepted design gap with proposed contract"