Governed Transitions Demo — XState engine + audit trail + frontend#5
Merged
Connorbelez merged 8 commits intomainfrom Mar 16, 2026
Merged
Governed Transitions Demo — XState engine + audit trail + frontend#5Connorbelez merged 8 commits intomainfrom
Connorbelez merged 8 commits intomainfrom
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts Zod schema for the Class diagram for updated validateZod Zod schemaclassDiagram
class ValidateZodInput {
+int count
+string email
}
class ValidateZodFunction {
+ValidateZodInput input
+void returns
}
ValidateZodFunction --> ValidateZodInput : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Merged
Contributor
|
Caution Review failedPull request was closed or merged during review Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a complete Governed Transitions demo: XState loan-application machine, Convex schema (entities, journal, effects_log), a transition engine with journaling, effect scheduling and audit-trail hashing, server-side effect handlers, frontend Command Center / Journal / Machine Inspector routes, UI adapters, tests (Vitest + Playwright), and documentation. Changes
Sequence DiagramsequenceDiagram
actor User
participant Client as Command Center
participant Backend as Transition Engine
participant Machine as XState Machine
participant Journal as Journal DB
participant Effects as Effect Scheduler
participant Audit as Audit Trail
User->>Client: trigger event (entityId, eventType, payload, source)
Client->>Backend: mutation transition(...)
Backend->>Backend: load entity (demo_gt_entities)
Backend->>Machine: hydrate state (resolveState)
Backend->>Machine: compute nextState = transition(event)
alt state changed
Backend->>Journal: insert journal entry (transitioned)
Backend->>Backend: persist new state & machineContext atomically
Backend->>Effects: schedule declared effects (scheduler.runAfter)
Backend->>Audit: schedule hash-chain write (hashChainJournalEntry)
Backend-->>Client: return TransitionResult(success, previousState, newState, effects)
Effects->>Journal: execute effect handlers -> insert demo_gt_effects_log
Audit->>Journal: write audit-layer entry
else rejected / no state change
Backend->>Journal: insert journal entry (rejected with reason)
Backend->>Audit: schedule hash-chain write
Backend-->>Client: return TransitionResult(success: false, reason)
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Owner
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Connorbelez
force-pushed
the
03-12-governed_transitions
branch
from
4b66a6e to
4e4a9fe
Compare
Connorbelez
force-pushed
the
graphite-base/5
branch
from
829f7a9 to
c8fe340
Compare
Connorbelez
force-pushed
the
03-12-governed_transitions
branch
from
4e4a9fe to
064f933
Compare
Owner
Author
|
@greptile please review |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @Connorbelez, your pull request is larger than the review limit of 150000 diff characters
Greptile SummaryThis PR delivers two things: (1) a targeted bug fix correcting the Zod email validator in Key changes:
Issues found:
Confidence Score: 4/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
Client([Client / UI]) -->|transition mutation| Engine[executeTransition]
Engine -->|1. Load entity| DB[(Convex DB)]
Engine -->|2. Lookup machine| Registry[machineRegistry]
Registry --> Machine[loanApplicationMachine]
Engine -->|3. resolveState| Machine
Engine -->|4. xstateTransition pure| Machine
Machine -->|nextState + actions| Engine
Engine -->|5a. No state change| JournalRej[Journal: rejected]
Engine -->|5b. State changed| PatchEntity[Patch entity status]
PatchEntity --> JournalTrans[Journal: transitioned]
JournalTrans -->|scheduleAfter 0| Effects[Effect handlers\nnotifyReviewer / notifyApplicant\nscheduleFunding / generateDocuments]
JournalTrans -->|scheduleAfter 0| HashChain[hashChainJournalEntry]
HashChain --> AuditTrail[auditTrail component\nhash-chain append]
Effects --> EffectsLog[(demo_gt_effects_log)]
style JournalRej fill:#fca5a5
style JournalTrans fill:#86efac
style AuditTrail fill:#c4b5fd
style Effects fill:#fdba74
Last reviewed commit: 064f933 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Connorbelez
force-pushed
the
03-12-governed_transitions
branch
from
064f933 to
99e9a02
Compare
There was a problem hiding this comment.
Pull request overview
This PR primarily adds a new Governed Transitions demo (Convex + TanStack Router UI + shared UI building blocks + tests/spec docs) and also includes a small fix to the fluent-convex demo’s Zod email validation.
Changes:
- Add Governed Transitions demo: Convex schema + XState machine + transition engine + effect scheduling + audit journaling + UI routes.
- Introduce/extend reusable UI components for read-only machine visualization and searchable journal logs.
- Add Playwright e2e coverage for the demo flows and read-only observer surfaces; fix Zod email validation.
Reviewed changes
Copilot reviewed 40 out of 42 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/routes/demo/governed-transitions/route.tsx | Layout route for the demo with nav tabs and outlet. |
| src/routes/demo/governed-transitions/index.tsx | Command Center UI for creating entities and dispatching transitions. |
| src/routes/demo/governed-transitions/journal.tsx | Journal page with stats + filters, uses the journal adapter. |
| src/routes/demo/governed-transitions/machine.tsx | Machine inspector page with highlight selector + transition table. |
| src/routes/demo/governed-transitions/_components/GovernedTransitionsMachineView.tsx | Adapter mapping machine snapshot/entities to N8nWorkflowBlock (read-only). |
| src/routes/demo/governed-transitions/_components/GovernedTransitionsJournalView.tsx | Adapter mapping journal entries to InteractiveLogsTable. |
| src/routeTree.gen.ts | Generated route tree updated for new governed-transitions routes. |
| src/components/ui/n8n-workflow-block-shadcnui.tsx | New/extended workflow visualization component with read-only mode. |
| src/components/ui/interactive-logs-table-shadcnui.tsx | New/extended searchable/filterable logs table component (now accepts logs props). |
| src/components/header.tsx | Adds “Governed Transitions” link under Platform demos. |
| specs/governed-transitions/tasks.md | Implementation task checklist for the governed-transitions demo. |
| specs/governed-transitions/gap-analysis.md | Spec-to-implementation gap analysis document. |
| specs/governed-transitions/design.md | Local design/spec document for the demo architecture and APIs. |
| specs/governed-transitions/chunks/manifest.md | Chunk manifest for implementation plan. |
| specs/governed-transitions/chunks/chunk-08-verification/tasks.md | Verification chunk tasks. |
| specs/governed-transitions/chunks/chunk-08-verification/context.md | Verification chunk context. |
| specs/governed-transitions/chunks/chunk-07-e2e-observer/tasks.md | Observer-surface e2e tasks. |
| specs/governed-transitions/chunks/chunk-07-e2e-observer/context.md | Observer-surface e2e context. |
| specs/governed-transitions/chunks/chunk-06-e2e-core/tasks.md | Core e2e tasks. |
| specs/governed-transitions/chunks/chunk-06-e2e-core/context.md | Core e2e context. |
| specs/governed-transitions/chunks/chunk-05-frontend-adapters/tasks.md | Frontend adapter tasks. |
| specs/governed-transitions/chunks/chunk-05-frontend-adapters/context.md | Frontend adapter context. |
| specs/governed-transitions/chunks/chunk-04-frontend-routes/tasks.md | Frontend routes tasks. |
| specs/governed-transitions/chunks/chunk-04-frontend-routes/context.md | Frontend routes context. |
| specs/governed-transitions/chunks/chunk-03-backend-queries/tasks.md | Backend queries/effects/tests tasks. |
| specs/governed-transitions/chunks/chunk-03-backend-queries/context.md | Backend queries/effects/tests context. |
| specs/governed-transitions/chunks/chunk-02-backend-engine/tasks.md | Backend engine tasks. |
| specs/governed-transitions/chunks/chunk-02-backend-engine/context.md | Backend engine context. |
| specs/governed-transitions/chunks/chunk-01-schema-machine/tasks.md | Schema/machine tasks. |
| specs/governed-transitions/chunks/chunk-01-schema-machine/context.md | Schema/machine context. |
| specs/governed-transitions/PRD.md | PRD snapshot for the governed-transitions demo. |
| package.json | Adds dependencies needed by the demo/components (e.g., framer-motion, xstate). |
| e2e/governed-transitions.spec.ts | Playwright e2e tests for core flows + observer read-only verification. |
| convex/schema.ts | Adds demo_gt_* tables and indexes for governed transitions. |
| convex/demo/machines/registry.ts | Machine registry for entity type → machine mapping. |
| convex/demo/machines/loanApplication.machine.ts | Pure XState v5 loan application state machine definition. |
| convex/demo/machines/tests/loanApplication.test.ts | Vitest coverage for machine transitions/guards/actions. |
| convex/demo/governedTransitionsEffects.ts | Internal mutations implementing effect handlers (writes to effects log). |
| convex/demo/governedTransitions.ts | Transition engine mutation + queries + internal mutations + helper utilities. |
| convex/demo/fluentConvex.ts | Fix Zod email validation to use z.string().email(). |
| convex/_generated/api.d.ts | Generated API typings updated for new Convex modules. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (6)
convex/demo/governedTransitionsEffects.ts (1)
10-64: Consider consolidating duplicate handler logic.All four mutations (
notifyReviewer,notifyApplicant,scheduleFunding,generateDocuments) share identical handler implementations. While keeping separate exports is reasonable for the registry dispatch pattern (seeeffectRegistryingovernedTransitions.ts:63-73), the handlers themselves could share a common implementation.If these are scaffolding for future differentiation (e.g.,
notifyReviewerwill eventually send real notifications), this is fine as-is. Otherwise, consider extracting a shared helper.♻️ Optional: Extract shared handler logic
+const createEffectHandler = () => async ( + ctx: { db: { insert: Function } }, + { entityId, journalEntryId, effectName }: { + entityId: Id<"demo_gt_entities">; + journalEntryId: Id<"demo_gt_journal">; + effectName: string; + } +) => { + await ctx.db.insert("demo_gt_effects_log", { + entityId, + journalEntryId, + effectName, + status: "completed", + scheduledAt: Date.now(), + completedAt: Date.now(), + }); +}; export const notifyReviewer = internalMutation({ args: effectArgs, - handler: async (ctx, { entityId, journalEntryId, effectName }) => { - await ctx.db.insert("demo_gt_effects_log", { - entityId, - journalEntryId, - effectName, - status: "completed", - scheduledAt: Date.now(), - completedAt: Date.now(), - }); - }, + handler: createEffectHandler(), });Apply the same pattern to the other three mutations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@convex/demo/governedTransitionsEffects.ts` around lines 10 - 64, The four internalMutation handlers (notifyReviewer, notifyApplicant, scheduleFunding, generateDocuments) duplicate the same DB insert logic; extract that logic into a single helper (e.g., logEffectHandler or buildEffectHandler) that accepts (ctx, { entityId, journalEntryId, effectName }) and performs the ctx.db.insert into "demo_gt_effects_log" with status/scheduledAt/completedAt, then update each internalMutation to reference that shared handler function instead of duplicating the insert code; keep the four exports and signatures the same so effectRegistry usage remains unchanged.e2e/governed-transitions.spec.ts (2)
391-408: This search test still passes if filtering is a no-op.The
CLOSErow already exists before the search, so re-checking that it is visible afterward does not prove the query narrowed the results. Also assert that a non-matching row disappears, or that the result set shrinks, after filling the input.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/governed-transitions.spec.ts` around lines 391 - 408, The test "journal search input filters entries" currently only verifies the matching "CLOSE" row is visible after filling the search, which doesn't prove filtering occurred; before typing into searchInput (page.getByPlaceholder(...)) capture the pre-search state (e.g., existence or count of a known non-matching row like the SUBMIT row found via page.getByText(/SUBMIT.*draft.*submitted/).first() or total result count) and after filling searchInput.fill("CLOSE") assert that the non-matching row is no longer visible or that the total number of results decreased (use the same selectors: page.getByText(...) or a shared result container) so the test fails if filtering is a no-op.
281-284: Scope this stats assertion to the actual stat cards.Line 282 just looks for the first
"5"on the page, so the test can still pass on unrelated text and miss a broken journal summary. Assert against the labeledTotal/Transitionedcards explicitly instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/governed-transitions.spec.ts` around lines 281 - 284, The assertion is too broad — replace the loose await expect(page.getByText("5").first()).toBeVisible(...) with a scoped check that targets the stat card elements for the "Total" and "Transitioned" labels (e.g., query the stat card container by its label text "Total" or "Transitioned" and then assert the numeric "5" is visible inside that container). Locate the test section in governed-transitions.spec.ts around the use of page.getByText("5").first() and update it to first find the specific stat card node by its label (or role/aria label) and then assert the number "5" within that node so unrelated page text cannot satisfy the expectation.convex/demo/machines/__tests__/loanApplication.test.ts (1)
138-163: Assert that rejected events emit no actions as well.These cases only prove the state value is unchanged. If a rejected event starts returning a marker action, this block still passes even though the rejection is no longer side-effect free.
🧪 Minimal pattern to apply in this block
- const [next] = tryTransition("draft", "APPROVE", VALID_DATA); + const [next, actions] = tryTransition("draft", "APPROVE", VALID_DATA); expect(next).toBe("draft"); + expect(actions).toEqual([]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@convex/demo/machines/__tests__/loanApplication.test.ts` around lines 138 - 163, The tests in the "invalid events leave state unchanged" block only assert the returned state but not that no side-effect actions were emitted; update each test (those calling tryTransition in this describe block) to also assert the actions result is empty (e.g., when calling tryTransition("draft","APPROVE", VALID_DATA) capture both return values and assert the actions array/collection is empty) so rejected events produce no actions; modify the tests that call tryTransition("draft", "REJECT", ...), tryTransition("submitted","FUND"), tryTransition("draft","CLOSE", ...), and tryTransition("under_review","SUBMIT") to include this additional assertion.src/routes/demo/governed-transitions/machine.tsx (1)
68-74: Use aninterfacefortransitionRowsitem shape.At Line 68-Line 74, prefer a named
interfaceover an inline object type to align with project typing conventions insrc/.As per coding guidelines, "Use
interfacefor defining object shapes in TypeScript files within TanStack Router codebase".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/demo/governed-transitions/machine.tsx` around lines 68 - 74, The inline object type for transitionRows should be replaced with a named interface: define an interface TransitionRow { fromState: string; event: string; guard: string; toState: string; actions: string; } and then change the transitionRows declaration to use Array<TransitionRow> (or TransitionRow[]) instead of the inline type; update any other uses of that shape (e.g., where items are pushed or typed) to reference TransitionRow so the file follows the project's interface convention.src/components/ui/interactive-logs-table-shadcnui.tsx (1)
28-32: Prefer an interface forFilters.This is an object shape in
src/, so it should follow the repo'sinterfaceconvention.As per coding guidelines, "Use interface for defining object shapes in TypeScript files within TanStack Router codebase".Suggested fix
-type Filters = { +interface Filters { level: string[]; service: string[]; status: string[]; -}; +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/interactive-logs-table-shadcnui.tsx` around lines 28 - 32, Replace the type alias `Filters` with an `interface` to match project convention: change the `type Filters = { level: string[]; service: string[]; status: string[]; };` alias to an `interface Filters { level: string[]; service: string[]; status: string[]; }` and ensure any usages of `Filters` (e.g., props or function signatures in the interactive logs component) remain valid; no other changes needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@convex/demo/governedTransitions.ts`:
- Around line 133-154: The rejected-branch currently inserts into
demo_gt_journal and returns before scheduling the hash-chain step, so audit
evidence is missing; after the ctx.db.insert(...) in the !transitioned branch,
call the same hash-chain scheduling helper used for successful commands
(hashChainJournalEntry) with the inserted journal data (entityType, entityId,
eventType, payload, previousState, newState: previousState, outcome: "rejected",
reason, source, machineVersion, timestamp) so rejected entries are queued for
hash-chaining just like successful ones, then return the same response object.
In `@convex/demo/machines/loanApplication.machine.ts`:
- Around line 53-55: The hasCompleteData validation currently allows
whitespace-only names (e.g., " ") because it only checks typeof
data.applicantName === "string" and length > 0; update hasCompleteData to trim
the applicantName (or use .trim().length) before checking length so
whitespace-only strings fail, and keep the existing type check and other
conditions (e.g., loanAmount) intact so SUBMIT cannot proceed with an
effectively empty name.
In `@specs/governed-transitions/chunks/chunk-01-schema-machine/context.md`:
- Around line 209-213: The fenced code block showing the file structure for
"convex/demo/ machines/ loanApplication.machine.ts" lacks a language specifier
which triggers MD040; update the triple-backtick fence at that block (the one
containing the directory tree) to include an appropriate language label such as
"text" or "bash" (e.g., change ``` to ```text) so the markdown linter recognizes
the fence language.
In `@specs/governed-transitions/chunks/chunk-02-backend-engine/context.md`:
- Around line 258-264: The fenced code block showing the directory listing
(starting with "convex/demo/" and listing governedTransitions.ts, machines/,
loanApplication.machine.ts, registry.ts) is missing a language identifier which
triggers MD040; update the opening fence to include a language (e.g., change ```
to ```text) so the block is fenced as ```text and leave the closing ```
unchanged.
In `@specs/governed-transitions/chunks/chunk-03-backend-queries/context.md`:
- Around line 322-329: The imports at the top of governedTransitions.ts use
"./_generated/*" which is wrong for files under convex/demo; update the module
specifiers for the generated imports so they point to the correct relative
_generated directory (e.g. change "./_generated/server" and "./_generated/api"
to the correct path such as "../_generated/server" and "../_generated/api" or
the appropriate relative path from convex/demo), keeping the imported symbols
unchanged (v, mutation, query, internalMutation, internal, components,
machineRegistry, EntityType, AuditTrail).
In `@specs/governed-transitions/chunks/chunk-04-frontend-routes/context.md`:
- Around line 112-113: Update the "All events" section wording to resolve the
contradiction: replace "gray/disabled" with language that clarifies invalid
events are visually muted (gray) and styled with cursor-not-allowed but are
still interactive (clickable) and will call transition to demonstrate rejection
logging; mention that native disabled attributes are not used so
tests/implementations should treat these as interactive elements (e.g., use
aria-disabled or a CSS-only disabled appearance) rather than wholly
non-clickable controls.
In `@specs/governed-transitions/chunks/chunk-05-frontend-adapters/context.md`:
- Around line 232-241: The fenced file-tree block in
specs/governed-transitions/chunks/chunk-05-frontend-adapters/context.md is
unlabeled and triggers MD040; update the opening fence from ``` to ```text
(i.e., mark the code fence with the language "text") for the block that lists
src/routes/demo/governed-transitions/ and its children (the block that contains
GovernedTransitionsJournalView.tsx, GovernedTransitionsMachineView.tsx,
route.tsx, index.tsx, journal.tsx, machine.tsx) so markdownlint stops flagging
it.
In `@specs/governed-transitions/design.md`:
- Around line 158-159: Update the guard description to match the implementation:
replace "Checks that the event payload contains a non-empty `applicantName`
string and a `loanAmount` number greater than 0" with a sentence stating that
the `hasCompleteData` guard (run on `SUBMIT`) validates those fields on
`context.data` (ensuring `context.data.applicantName` is a non-empty string and
`context.data.loanAmount` is a number > 0), making clear it reads from
`context.data` rather than the event payload.
- Around line 16-18: The design doc's demo note omits the optional transition
arg `ip` but the backend transition args include `ip?: string`; update the docs
to match the implemented contract by adding `ip?: string` to the demo metadata
shape (or explicitly state that the demo still sends an optional `ip` field even
if not used), mentioning the `ip` field alongside transition args/metadata in
specs/governed-transitions/design.md so the documented shape matches the
backend's `ip` behavior.
In `@src/components/ui/interactive-logs-table-shadcnui.tsx`:
- Around line 463-475: The filter toggle Button is icon-only and lacks an
accessible name and expanded state; update the Button (where setShowFilters,
showFilters, Badge, and activeFilters are used) to include an accessible name
and state by adding attributes such as aria-label="Toggle filters" (or
visually-hidden text) and aria-expanded={showFilters}, and also add
aria-controls pointing to the filter panel's id (ensure the panel has a matching
id like "filters-panel"); this provides a stable name and exposes
expanded/collapsed state to assistive tech.
In `@src/components/ui/n8n-workflow-block-shadcnui.tsx`:
- Around line 167-180: The component currently mixes prop-backed and internal
state (propNodes/propConnections vs internalNodes/internalConnections) so
interactive edits only update internal state and disappear when props are
supplied; change the editing logic in N8nWorkflowBlock so it fully supports
either controlled or uncontrolled usage: treat presence of
propNodes/propConnections as controlled mode and require corresponding change
callbacks (e.g., onNodesChange, onConnectionsChange) — update all
drag/add/remove handlers to call the provided callbacks when in controlled mode
and only call setInternalNodes/setInternalConnections when uncontrolled, and add
useEffect syncs to update internal state from props if you need to support
switching modes. Reference N8nWorkflowBlock, propNodes, propConnections,
internalNodes, internalConnections, setInternalNodes, setInternalConnections,
and ensure every place that mutates nodes/connections (dragging, adding
connections, removing nodes) uses this conditional update pattern.
In `@src/routes/demo/governed-transitions/index.tsx`:
- Around line 84-93: The current form only checks for non-empty label and
loanAmount strings before calling createEntityMut; add a guard that converts
loanAmount to a number (e.g., const amt = Number(loanAmount.trim())), then check
Number.isFinite(amt) && amt > 0 and if it fails call setError with a clear
message (e.g., "Loan amount must be a positive number") and return early; ensure
createEntityMut is called with the validated numeric amt and keep applicantName
logic unchanged.
- Around line 192-203: The Button handlers call seedMut({}) and
runLifecycleMut({}) directly which can reject and bypass your setError flow;
wrap both calls in async handlers (or use .then/.catch) so you await the
mutation results and catch failures, then call setError with a helpful message
(and optionally log the error) inside the catch block; update the onClick for
the Seed button (seedMut) and the Run Full Lifecycle button (runLifecycleMut) to
use try/catch around the awaited mutation and invoke setError(...) on failure to
surface errors to the UI.
In `@src/routes/demo/governed-transitions/journal.tsx`:
- Around line 29-30: The stats cards are fetched globally via stats =
useQuery(api.demo.governedTransitions.getJournalStats) and therefore don’t
reflect entityFilter and outcomeFilter; update the stats query to accept the
same filters used by listEntities (entityFilter and outcomeFilter) by calling
useQuery(api.demo.governedTransitions.getJournalStats, { input: { entityFilter,
outcomeFilter } }) or, alternatively, derive the counts from entities.data
(reduce/filter by entityFilter/outcomeFilter) and render those values; ensure
the new query uses the same reactive filter variables as listEntities so the
cards update when filters change.
---
Nitpick comments:
In `@convex/demo/governedTransitionsEffects.ts`:
- Around line 10-64: The four internalMutation handlers (notifyReviewer,
notifyApplicant, scheduleFunding, generateDocuments) duplicate the same DB
insert logic; extract that logic into a single helper (e.g., logEffectHandler or
buildEffectHandler) that accepts (ctx, { entityId, journalEntryId, effectName })
and performs the ctx.db.insert into "demo_gt_effects_log" with
status/scheduledAt/completedAt, then update each internalMutation to reference
that shared handler function instead of duplicating the insert code; keep the
four exports and signatures the same so effectRegistry usage remains unchanged.
In `@convex/demo/machines/__tests__/loanApplication.test.ts`:
- Around line 138-163: The tests in the "invalid events leave state unchanged"
block only assert the returned state but not that no side-effect actions were
emitted; update each test (those calling tryTransition in this describe block)
to also assert the actions result is empty (e.g., when calling
tryTransition("draft","APPROVE", VALID_DATA) capture both return values and
assert the actions array/collection is empty) so rejected events produce no
actions; modify the tests that call tryTransition("draft", "REJECT", ...),
tryTransition("submitted","FUND"), tryTransition("draft","CLOSE", ...), and
tryTransition("under_review","SUBMIT") to include this additional assertion.
In `@e2e/governed-transitions.spec.ts`:
- Around line 391-408: The test "journal search input filters entries" currently
only verifies the matching "CLOSE" row is visible after filling the search,
which doesn't prove filtering occurred; before typing into searchInput
(page.getByPlaceholder(...)) capture the pre-search state (e.g., existence or
count of a known non-matching row like the SUBMIT row found via
page.getByText(/SUBMIT.*draft.*submitted/).first() or total result count) and
after filling searchInput.fill("CLOSE") assert that the non-matching row is no
longer visible or that the total number of results decreased (use the same
selectors: page.getByText(...) or a shared result container) so the test fails
if filtering is a no-op.
- Around line 281-284: The assertion is too broad — replace the loose await
expect(page.getByText("5").first()).toBeVisible(...) with a scoped check that
targets the stat card elements for the "Total" and "Transitioned" labels (e.g.,
query the stat card container by its label text "Total" or "Transitioned" and
then assert the numeric "5" is visible inside that container). Locate the test
section in governed-transitions.spec.ts around the use of
page.getByText("5").first() and update it to first find the specific stat card
node by its label (or role/aria label) and then assert the number "5" within
that node so unrelated page text cannot satisfy the expectation.
In `@src/components/ui/interactive-logs-table-shadcnui.tsx`:
- Around line 28-32: Replace the type alias `Filters` with an `interface` to
match project convention: change the `type Filters = { level: string[]; service:
string[]; status: string[]; };` alias to an `interface Filters { level:
string[]; service: string[]; status: string[]; }` and ensure any usages of
`Filters` (e.g., props or function signatures in the interactive logs component)
remain valid; no other changes needed.
In `@src/routes/demo/governed-transitions/machine.tsx`:
- Around line 68-74: The inline object type for transitionRows should be
replaced with a named interface: define an interface TransitionRow { fromState:
string; event: string; guard: string; toState: string; actions: string; } and
then change the transitionRows declaration to use Array<TransitionRow> (or
TransitionRow[]) instead of the inline type; update any other uses of that shape
(e.g., where items are pushed or typed) to reference TransitionRow so the file
follows the project's interface convention.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e9b937f1-66dd-4700-9ad7-4e95719122ac
⛔ Files ignored due to path filters (2)
bun.lockis excluded by!**/*.lockconvex/_generated/api.d.tsis excluded by!**/_generated/**
📒 Files selected for processing (40)
convex/demo/fluentConvex.tsconvex/demo/governedTransitions.tsconvex/demo/governedTransitionsEffects.tsconvex/demo/machines/__tests__/loanApplication.test.tsconvex/demo/machines/loanApplication.machine.tsconvex/demo/machines/registry.tsconvex/schema.tse2e/governed-transitions.spec.tspackage.jsonspecs/governed-transitions/PRD.mdspecs/governed-transitions/chunks/chunk-01-schema-machine/context.mdspecs/governed-transitions/chunks/chunk-01-schema-machine/tasks.mdspecs/governed-transitions/chunks/chunk-02-backend-engine/context.mdspecs/governed-transitions/chunks/chunk-02-backend-engine/tasks.mdspecs/governed-transitions/chunks/chunk-03-backend-queries/context.mdspecs/governed-transitions/chunks/chunk-03-backend-queries/tasks.mdspecs/governed-transitions/chunks/chunk-04-frontend-routes/context.mdspecs/governed-transitions/chunks/chunk-04-frontend-routes/tasks.mdspecs/governed-transitions/chunks/chunk-05-frontend-adapters/context.mdspecs/governed-transitions/chunks/chunk-05-frontend-adapters/tasks.mdspecs/governed-transitions/chunks/chunk-06-e2e-core/context.mdspecs/governed-transitions/chunks/chunk-06-e2e-core/tasks.mdspecs/governed-transitions/chunks/chunk-07-e2e-observer/context.mdspecs/governed-transitions/chunks/chunk-07-e2e-observer/tasks.mdspecs/governed-transitions/chunks/chunk-08-verification/context.mdspecs/governed-transitions/chunks/chunk-08-verification/tasks.mdspecs/governed-transitions/chunks/manifest.mdspecs/governed-transitions/design.mdspecs/governed-transitions/gap-analysis.mdspecs/governed-transitions/tasks.mdsrc/components/header.tsxsrc/components/ui/interactive-logs-table-shadcnui.tsxsrc/components/ui/n8n-workflow-block-shadcnui.tsxsrc/routeTree.gen.tssrc/routes/demo/governed-transitions/_components/GovernedTransitionsJournalView.tsxsrc/routes/demo/governed-transitions/_components/GovernedTransitionsMachineView.tsxsrc/routes/demo/governed-transitions/index.tsxsrc/routes/demo/governed-transitions/journal.tsxsrc/routes/demo/governed-transitions/machine.tsxsrc/routes/demo/governed-transitions/route.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (5)
src/components/ui/interactive-logs-table-shadcnui.tsx (1)
463-475:⚠️ Potential issue | 🟠 MajorAdd accessible name and expanded state to the filter toggle.
The icon-only toggle still lacks a stable accessible label and relation to the controlled panel.
Proposed fix
<Button variant={showFilters ? "default" : "outline"} size="sm" onClick={() => setShowFilters((current) => !current)} className="relative" + aria-label={showFilters ? "Hide filters" : "Show filters"} + aria-expanded={showFilters} + aria-controls="interactive-logs-filters" > <Filter className="h-4 w-4" /> @@ <motion.div + id="interactive-logs-filters" key="filters" initial={{ width: 0, opacity: 0 }} animate={{ width: 280, opacity: 1 }}Also applies to: 483-490
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/interactive-logs-table-shadcnui.tsx` around lines 463 - 475, The filter toggle Button (the one rendering <Filter />, activeFilters and toggling setShowFilters/showFilters) is missing accessible name and expanded state; update the Button to include an appropriate aria-label (e.g., "Toggle filters"), aria-expanded set to the showFilters boolean, and aria-controls pointing to the id of the filter panel it toggles (create or reference a stable id on the panel component); apply the same changes to the other similar Button instance mentioned (lines 483-490) so screen readers can identify the control and its relationship to the controlled panel.specs/governed-transitions/chunks/chunk-04-frontend-routes/context.md (1)
112-113:⚠️ Potential issue | 🟡 MinorClarify invalid-event controls as visually disabled but still interactive.
This still reads as contradictory (“disabled” but clickable), which can cause inconsistent UI/test behavior.
Proposed wording fix
- - **All events** section: all 9 event types shown. Valid ones are green, invalid ones are gray/disabled with `cursor-not-allowed`. Clicking an invalid one still calls `transition` to demonstrate rejection logging. + - **All events** section: all 9 event types shown. Valid ones are green; invalid ones are visually muted (gray) with `cursor-not-allowed` but remain clickable (no native `disabled`). Clicking an invalid one still calls `transition` to demonstrate rejection logging.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@specs/governed-transitions/chunks/chunk-04-frontend-routes/context.md` around lines 112 - 113, Reword the "All events" description to avoid saying "disabled" while making behavior explicit: state that invalid event buttons are visually styled as disabled (gray, cursor-not-allowed) but remain interactive and will call the transition function to demonstrate rejection logging; reference the "All events" section, the `cursor-not-allowed` visual style, and the `transition` call so readers/tests understand the visual-only disabled affordance and the still-invoked interaction.src/components/ui/n8n-workflow-block-shadcnui.tsx (1)
167-180:⚠️ Potential issue | 🟠 MajorProp-backed mode is half-controlled and drops user edits.
When
nodes/connectionsprops are supplied, drag/add still update only internal state, but render reads props. Edits won’t persist visually.Suggested safeguard (disable editing in prop-backed mode)
export function N8nWorkflowBlock({ nodes: propNodes, connections: propConnections, readOnly = false, title = "Workflow Builder", activeNodeId, }: N8nWorkflowBlockProps) { + const hasExternalGraph = + propNodes !== undefined || propConnections !== undefined; + const canEdit = !readOnly && !hasExternalGraph; + const [internalNodes, setInternalNodes] = useState<WorkflowNode[]>(initialNodes); @@ const handleDragStart = (nodeId: string) => { - if (readOnly) return; + if (!canEdit) return; @@ const handleDrag = (nodeId: string, { offset }: PanInfo) => { - if (readOnly) return; + if (!canEdit) return; @@ const addNode = () => { - if (readOnly) return; + if (!canEdit) return; @@ - {!readOnly && ( + {canEdit && ( @@ - drag={!readOnly} + drag={canEdit} @@ - className={readOnly ? "absolute" : "absolute cursor-grab"} + className={canEdit ? "absolute cursor-grab" : "absolute"} @@ - whileHover={readOnly ? undefined : { scale: 1.02 }} - whileDrag={readOnly ? undefined : { scale: 1.05, zIndex: 50, cursor: "grabbing" }} + whileHover={canEdit ? { scale: 1.02 } : undefined} + whileDrag={canEdit ? { scale: 1.05, zIndex: 50, cursor: "grabbing" } : undefined} @@ - {!readOnly && ( + {canEdit && (Also applies to: 211-244, 252-290, 307-318, 363-387, 461-465
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/n8n-workflow-block-shadcnui.tsx` around lines 167 - 180, N8nWorkflowBlock is half-controlled: when propNodes/propConnections are provided the component renders those props but all mutating handlers still update internalNodes/internalConnections, so user edits disappear; fix by detecting "prop-backed" mode (propNodes !== undefined || propConnections !== undefined) and making mutating handlers (any functions that call setInternalNodes or setInternalConnections—e.g., node drag/add handlers, connection create/delete handlers) no-ops unless corresponding onNodesChange/onConnectionsChange callbacks are passed, or alternatively disable editing UI when in prop-backed mode; update logic around the nodes/connections variables (currently set via const nodes = propNodes ?? internalNodes and similar) to use this guard so state updates won’t be applied when props control the data.specs/governed-transitions/chunks/chunk-01-schema-machine/context.md (1)
209-213:⚠️ Potential issue | 🟡 MinorAdd a language tag to the fenced file-structure block.
The fence still has no language specifier, so markdown lint will continue to flag it.
Proposed fix
-``` +```text convex/demo/ machines/ loanApplication.machine.ts — Pure XState v5 machine definition (zero Convex imports)</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@specs/governed-transitions/chunks/chunk-01-schema-machine/context.mdaround
lines 209 - 213, Add a language tag to the fenced file-structure block by
changing the opening fence fromtotext so the block starting with
"convex/demo/" and the nested "machines/ loanApplication.machine.ts — Pure
XState v5 machine definition" is marked as plain text and satisfies the markdown
linter; update the fence around that block accordingly.</details> </blockquote></details> <details> <summary>convex/demo/governedTransitions.ts (1)</summary><blockquote> `133-154`: _⚠️ Potential issue_ | _🟠 Major_ **Rejected transitions bypass hash-chain audit handoff.** The rejected path inserts a journal entry but returns before scheduling `hashChainJournalEntry`, while successful transitions (lines 197-202) are hash-chained. This creates incomplete audit evidence for compliance. <details> <summary>Suggested fix</summary> ```diff if (!transitioned) { // 5a. Command rejected — log to journal and return - await ctx.db.insert("demo_gt_journal", { + const journalId = await ctx.db.insert("demo_gt_journal", { entityType, entityId, eventType, payload: payload ?? undefined, previousState, newState: previousState, outcome: "rejected", reason: `No valid transition for ${eventType} from ${previousState}`, source, machineVersion: machineDef.config.id ?? "unknown", timestamp: Date.now(), }); + + await ctx.scheduler.runAfter( + 0, + internal.demo.governedTransitions.hashChainJournalEntry, + { journalEntryId: journalId } + ); return { success: false, ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@convex/demo/governedTransitions.ts` around lines 133 - 154, Rejected transitions currently insert a journal row into demo_gt_journal then return without scheduling the audit handoff; update the rejected branch in governedTransitions (the block that calls ctx.db.insert when !transitioned) to also schedule/hash-chain the same journal entry using the same mechanism as the successful path (call the hashChainJournalEntry scheduling helper or equivalent used in lines ~197-202) before returning so rejected outcomes get the same hash-chain audit handoff and metadata (include the journal id or payload used by hashChainJournalEntry). ``` </details> </blockquote></details> </blockquote></details> <details> <summary>🧹 Nitpick comments (8)</summary><blockquote> <details> <summary>specs/governed-transitions/chunks/chunk-08-verification/context.md (2)</summary><blockquote> `57-83`: **Add evidence capture to the verification checklist.** The checklist is good, but adding an `Evidence` column (test name, screenshot, log, or file/line reference) will make sign-off auditable and reduce re-validation churn. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@specs/governed-transitions/chunks/chunk-08-verification/context.md` around lines 57 - 83, Add an "Evidence" column to the verification checklist table (the markdown table starting with "| Criterion | Pass? |") in context.md: update the header row to include "| Evidence |", update the separator row accordingly, and append an extra empty cell for Evidence to every existing criterion row (e.g., F-1..F-9, REQ-1..REQ-10 and the other surface/check rows) so each line becomes "| ... | | |" ready to record test names, screenshots, logs, or file/line references. ``` </details> --- `40-47`: **Broaden read-only verification beyond `useMutation` checks.** At Line 40 and Line 46, checking only for `useMutation` can miss other mutative paths (e.g., wrapped hooks or direct action calls). Prefer wording that verifies *no mutative actions are reachable from these routes*, then keep `useMutation` as one concrete check. <details> <summary>♻️ Suggested doc tweak</summary> ```diff -- Check the component code — ensure no `useMutation` calls in journal.tsx +- Check the component code — ensure no mutative actions are reachable from `journal.tsx` + (e.g., no `useMutation`, no mutative action wrappers, no direct mutation dispatch paths) -- Check the component code — ensure no `useMutation` calls in machine.tsx, and that N8nWorkflowBlock is rendered with `readOnly={true}` +- Check the component code — ensure no mutative actions are reachable from `machine.tsx` + (e.g., no `useMutation`, no mutative action wrappers, no direct mutation dispatch paths), + and that `N8nWorkflowBlock` is rendered with `readOnly={true}`. ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@specs/governed-transitions/chunks/chunk-08-verification/context.md` around lines 40 - 47, The current verification text only looks for useMutation calls but can miss other mutative paths; update the checks for journal.tsx and machine.tsx to require that no mutative actions are reachable from these routes (not just absence of useMutation), e.g., explicitly verify there are no mutation-related hooks or wrapped hooks, no direct action calls that perform state/server mutations, and that N8nWorkflowBlock is rendered with readOnly={true} (for machine.tsx) and any interactive handlers (drag/drop, add/edit node/edge, save actions) are disabled or absent; keep useMutation as one concrete check but broaden wording to include these other mutative entry points and to assert handlers/props are set to read-only/no-op. ``` </details> </blockquote></details> <details> <summary>e2e/governed-transitions.spec.ts (2)</summary><blockquote> `60-74`: **Consider a more specific locator for entity selection.** The locator chain filtering by `hasText` on a `div` and then getting the first button can be fragile if the DOM structure changes or multiple entities share similar text patterns. <details> <summary>♻️ Suggested improvement using data-testid</summary> If feasible, add a `data-testid` attribute to entity cards in the UI component: ```diff - const entityCard = page - .locator("div") - .filter({ hasText: label }) - .getByRole("button") - .first(); + // Prefer data-testid for more robust selection + const entityCard = page.getByTestId(`entity-card-${label.toLowerCase().replace(/\s+/g, '-')}`); + // Fallback if data-testid not available: + // const entityCard = page.locator(`[data-entity-label="${label}"]`).getByRole('button').first(); ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@e2e/governed-transitions.spec.ts` around lines 60 - 74, The selectEntity helper uses a fragile locator chain (page.locator("div").filter({ hasText: label }).getByRole("button").first()) which can break if DOM changes; update it to use a more specific selector (preferably a data-testid on the entity card) or Playwright role-based lookup with an accessible name, e.g., target the button by role and name or by a unique attribute like data-testid; change references to entityCard in selectEntity and keep the DATA_LOAD_TIMEOUT assertion, ensuring the new locator remains scoped to the same Page instance and clicks the intended element. ``` </details> --- `368-388`: **Extract duplicate mutation labels array to a shared constant.** The `mutationLabels` array is duplicated in both T-045A (lines 368-382) and T-045B (lines 448-462). Extract to a file-level constant for maintainability. <details> <summary>♻️ Proposed refactor</summary> ```diff +// At the top of the file, after the helpers +const MUTATION_BUTTON_LABELS = [ + "Create Application", + "Reset Demo", + "Seed Data", + "Run Full Lifecycle", + "SUBMIT", + "APPROVE", + "REJECT", + "FUND", + "CLOSE", + "ASSIGN_REVIEWER", + "REQUEST_INFO", + "RESUBMIT", + "REOPEN", +] as const; // Then in both tests: - const mutationLabels = [ - "Create Application", - ... - ]; - - for (const label of mutationLabels) { + for (const label of MUTATION_BUTTON_LABELS) { ``` </details> Also applies to: 448-468 <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@e2e/governed-transitions.spec.ts` around lines 368 - 388, Extract the duplicated mutationLabels array into a single file-level constant (e.g., const MUTATION_LABELS = [...]) and replace both in-test occurrences (used in the T-045A and T-045B test blocks where mutationLabels is iterated and passed to page.getByRole) with that constant; ensure the new constant is defined near the top of the spec so both tests can reference it and remove the local mutationLabels declarations from each test. ``` </details> </blockquote></details> <details> <summary>convex/demo/governedTransitionsEffects.ts (1)</summary><blockquote> `10-64`: **Consider extracting the shared handler logic.** All four mutations have identical handler implementations. While acceptable for a demo, extracting a shared handler would reduce duplication and make future changes easier. <details> <summary>♻️ Proposed refactor</summary> ```diff +const createEffectHandler = () => async ( + ctx: { db: { insert: (table: string, doc: unknown) => Promise<unknown> } }, + { entityId, journalEntryId, effectName }: { + entityId: Id<"demo_gt_entities">; + journalEntryId: Id<"demo_gt_journal">; + effectName: string; + } +) => { + await ctx.db.insert("demo_gt_effects_log", { + entityId, + journalEntryId, + effectName, + status: "completed", + scheduledAt: Date.now(), + completedAt: Date.now(), + }); +}; export const notifyReviewer = internalMutation({ args: effectArgs, - handler: async (ctx, { entityId, journalEntryId, effectName }) => { - await ctx.db.insert("demo_gt_effects_log", { - entityId, - journalEntryId, - effectName, - status: "completed", - scheduledAt: Date.now(), - completedAt: Date.now(), - }); - }, + handler: createEffectHandler(), }); // ... apply same pattern to other mutations ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@convex/demo/governedTransitionsEffects.ts` around lines 10 - 64, All four internalMutation handlers (notifyReviewer, notifyApplicant, scheduleFunding, generateDocuments) duplicate the same insert logic; extract that into a single helper (e.g., async function logEffect(ctx, { entityId, journalEntryId, effectName }) or a factory like createEffectHandler(effectName)) and have each mutation reference that shared handler instead of repeating the insert block; update notifyReviewer, notifyApplicant, scheduleFunding and generateDocuments to call the helper (or use the factory to produce the handler) and remove the duplicated insert bodies. ``` </details> </blockquote></details> <details> <summary>convex/demo/governedTransitions.ts (3)</summary><blockquote> `417-424`: **In-memory sort could be replaced with index ordering.** The query collects all entities then sorts in memory. If the `createdAt` field has an index, you could use `.order("desc")` on that index. Alternatively, Convex's `_creationTime` system field is automatically indexed and could be used if acceptable as a substitute. <details> <summary>Example using _creationTime</summary> ```diff export const listEntities = query({ args: {}, handler: async (ctx) => { - const entities = await ctx.db.query("demo_gt_entities").collect(); - // Order by creation time descending - return entities.sort((a, b) => b.createdAt - a.createdAt); + return await ctx.db + .query("demo_gt_entities") + .order("desc") + .collect(); }, }); ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@convex/demo/governedTransitions.ts` around lines 417 - 424, The handler in listEntities currently fetches all rows then sorts in-memory; change the query to use the index ordering instead: call ctx.db.query("demo_gt_entities").order("createdAt", "desc").collect() if createdAt is indexed, or use the Convex system index via .order("_creationTime", "desc") if createdAt is not indexed, and return that result (remove the manual .sort step) so ordering is done by the DB/index. ``` </details> --- `218-218`: **Consider constraining `payload` type.** `v.any()` bypasses type safety. If payload structures are predictable, consider using a union of expected shapes or `v.record(v.string(), v.any())` for slightly better constraints. However, if payloads are truly arbitrary per event type, this may be acceptable. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@convex/demo/governedTransitions.ts` at line 218, The payload field currently uses v.optional(v.any()), which bypasses validation; replace v.any() with a tighter schema: either a union of expected payload shapes (e.g., v.union(...)) matching each event type or, if keys are dynamic but values are arbitrary, use v.record(v.string(), v.any()); update the schema where payload is defined (the payload: v.optional(v.any()) entry) and adjust any callers/parsers to conform to the chosen shape so payloads are validated appropriately. ``` </details> --- `468-487`: **Counting all journal entries on every call is inefficient at scale.** The query fetches all records to compute counts. For production workloads, consider using the `convex-dev-aggregate` component for efficient pre-computed counts, sums, and analytics over large datasets. Based on learnings, this component is recommended for aggregate analytics. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@convex/demo/governedTransitions.ts` around lines 468 - 487, getJournalStats currently loads all demo_gt_journal rows to count outcomes which will not scale; replace this full-scan with an aggregate or pre-computed counters: use the convex aggregate API (or the project’s convex-dev-aggregate component) to query counts for outcome="transitioned" and outcome!="transitioned" (or maintain a small stats row updated in the transition/reject mutation handlers) instead of ctx.db.query(...).collect(); update getJournalStats to call the aggregate endpoint or read the precomputed stats table and return total/transitioned/rejected based on those aggregated values. ``` </details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Inline comments:
In@specs/governed-transitions/chunks/chunk-02-backend-engine/tasks.md:
- Around line 1-7: The task list in this chunk (T-010 through T-014) is
inconsistent with the main specs: update the completion checkboxes in this file
so they match the main specs' status (either mark T-010..T-014 as complete "[x]"
to match specs/governed-transitions/tasks.md or consolidate both files into a
single source of truth), and ensure the task identifiers (T-010, T-011, T-012,
T-013, T-014) and their descriptions remain unchanged so downstream references
like machineRegistry, governedTransitions.transition, createEntity,
seedEntities, and runFullLifecycle still match the implementation.In
@specs/governed-transitions/tasks.md:
- Around line 40-47: The docs reference the wrong test file path: replace the
string "tests/e2e/governed-transitions.spec.ts" in
specs/governed-transitions/tasks.md with the actual test location
"e2e/governed-transitions.spec.ts" (or alternatively update the repo layout to
move the spec into tests/e2e/); update any other occurrences of that path in the
same file so the documented path matches the real file path used by the
Playwright setup (refer to the exact path strings
"tests/e2e/governed-transitions.spec.ts" and "e2e/governed-transitions.spec.ts"
when making the change).In
@src/components/ui/n8n-workflow-block-shadcnui.tsx:
- Around line 185-193: The contentSize state initializer uses Math.max over
sourceNodes which yields -Infinity for an empty array; update the initializer in
the contentSize / setContentSize logic (referencing propNodes, initialNodes,
NODE_WIDTH, NODE_HEIGHT) to handle empty sourceNodes by providing safe defaults
(e.g., treat empty mapped arrays as [0] or use Math.max(0, ...mappedValues)) so
width/height compute to valid numbers (like at least 50 offsets) instead of
-Infinity.In
@src/routes/demo/governed-transitions/_components/GovernedTransitionsJournalView.tsx:
- Line 29: The duration field currently uses an empty string (duration: "")
which renders an empty cell; update the default/placeholder value in the
GovernedTransitionsJournalView component (where duration is set) to a visible
placeholder like "—" (or a shared constant e.g., DURATION_PLACEHOLDER) so
empty/unknown durations render consistently and improve readability.- Around line 15-20: The subtitle currently uses unfiltered getJournalStats,
causing mismatched counts; instead derive stats from the filtered journal query
result. In GovernedTransitionsJournalView, stop using
api.demo.governedTransitions.getJournalStats for the header/subtitle and compute
the needed counts (total/filtered/other metrics) from journal.data (the result
of useQuery(api.demo.governedTransitions.getJournal, { entityId, outcome })) and
use those values for rendering; also update the other occurrence that references
stats (the second use around lines 39-41) to read from the filtered journal
result as well. Ensure any null/undefined guards around journal.data are
preserved.
Duplicate comments:
In@convex/demo/governedTransitions.ts:
- Around line 133-154: Rejected transitions currently insert a journal row into
demo_gt_journal then return without scheduling the audit handoff; update the
rejected branch in governedTransitions (the block that calls ctx.db.insert when
!transitioned) to also schedule/hash-chain the same journal entry using the same
mechanism as the successful path (call the hashChainJournalEntry scheduling
helper or equivalent used in lines ~197-202) before returning so rejected
outcomes get the same hash-chain audit handoff and metadata (include the journal
id or payload used by hashChainJournalEntry).In
@specs/governed-transitions/chunks/chunk-01-schema-machine/context.md:
- Around line 209-213: Add a language tag to the fenced file-structure block by
changing the opening fence fromtotext so the block starting with
"convex/demo/" and the nested "machines/ loanApplication.machine.ts — Pure
XState v5 machine definition" is marked as plain text and satisfies the markdown
linter; update the fence around that block accordingly.In
@specs/governed-transitions/chunks/chunk-04-frontend-routes/context.md:
- Around line 112-113: Reword the "All events" description to avoid saying
"disabled" while making behavior explicit: state that invalid event buttons are
visually styled as disabled (gray, cursor-not-allowed) but remain interactive
and will call the transition function to demonstrate rejection logging;
reference the "All events" section, thecursor-not-allowedvisual style, and
thetransitioncall so readers/tests understand the visual-only disabled
affordance and the still-invoked interaction.In
@src/components/ui/interactive-logs-table-shadcnui.tsx:
- Around line 463-475: The filter toggle Button (the one rendering ,
activeFilters and toggling setShowFilters/showFilters) is missing accessible
name and expanded state; update the Button to include an appropriate aria-label
(e.g., "Toggle filters"), aria-expanded set to the showFilters boolean, and
aria-controls pointing to the id of the filter panel it toggles (create or
reference a stable id on the panel component); apply the same changes to the
other similar Button instance mentioned (lines 483-490) so screen readers can
identify the control and its relationship to the controlled panel.In
@src/components/ui/n8n-workflow-block-shadcnui.tsx:
- Around line 167-180: N8nWorkflowBlock is half-controlled: when
propNodes/propConnections are provided the component renders those props but all
mutating handlers still update internalNodes/internalConnections, so user edits
disappear; fix by detecting "prop-backed" mode (propNodes !== undefined ||
propConnections !== undefined) and making mutating handlers (any functions that
call setInternalNodes or setInternalConnections—e.g., node drag/add handlers,
connection create/delete handlers) no-ops unless corresponding
onNodesChange/onConnectionsChange callbacks are passed, or alternatively disable
editing UI when in prop-backed mode; update logic around the nodes/connections
variables (currently set via const nodes = propNodes ?? internalNodes and
similar) to use this guard so state updates won’t be applied when props control
the data.
Nitpick comments:
In@convex/demo/governedTransitions.ts:
- Around line 417-424: The handler in listEntities currently fetches all rows
then sorts in-memory; change the query to use the index ordering instead: call
ctx.db.query("demo_gt_entities").order("createdAt", "desc").collect() if
createdAt is indexed, or use the Convex system index via .order("_creationTime",
"desc") if createdAt is not indexed, and return that result (remove the manual
.sort step) so ordering is done by the DB/index.- Line 218: The payload field currently uses v.optional(v.any()), which bypasses
validation; replace v.any() with a tighter schema: either a union of expected
payload shapes (e.g., v.union(...)) matching each event type or, if keys are
dynamic but values are arbitrary, use v.record(v.string(), v.any()); update the
schema where payload is defined (the payload: v.optional(v.any()) entry) and
adjust any callers/parsers to conform to the chosen shape so payloads are
validated appropriately.- Around line 468-487: getJournalStats currently loads all demo_gt_journal rows
to count outcomes which will not scale; replace this full-scan with an aggregate
or pre-computed counters: use the convex aggregate API (or the project’s
convex-dev-aggregate component) to query counts for outcome="transitioned" and
outcome!="transitioned" (or maintain a small stats row updated in the
transition/reject mutation handlers) instead of ctx.db.query(...).collect();
update getJournalStats to call the aggregate endpoint or read the precomputed
stats table and return total/transitioned/rejected based on those aggregated
values.In
@convex/demo/governedTransitionsEffects.ts:
- Around line 10-64: All four internalMutation handlers (notifyReviewer,
notifyApplicant, scheduleFunding, generateDocuments) duplicate the same insert
logic; extract that into a single helper (e.g., async function logEffect(ctx, {
entityId, journalEntryId, effectName }) or a factory like
createEffectHandler(effectName)) and have each mutation reference that shared
handler instead of repeating the insert block; update notifyReviewer,
notifyApplicant, scheduleFunding and generateDocuments to call the helper (or
use the factory to produce the handler) and remove the duplicated insert bodies.In
@e2e/governed-transitions.spec.ts:
- Around line 60-74: The selectEntity helper uses a fragile locator chain
(page.locator("div").filter({ hasText: label }).getByRole("button").first())
which can break if DOM changes; update it to use a more specific selector
(preferably a data-testid on the entity card) or Playwright role-based lookup
with an accessible name, e.g., target the button by role and name or by a unique
attribute like data-testid; change references to entityCard in selectEntity and
keep the DATA_LOAD_TIMEOUT assertion, ensuring the new locator remains scoped to
the same Page instance and clicks the intended element.- Around line 368-388: Extract the duplicated mutationLabels array into a single
file-level constant (e.g., const MUTATION_LABELS = [...]) and replace both
in-test occurrences (used in the T-045A and T-045B test blocks where
mutationLabels is iterated and passed to page.getByRole) with that constant;
ensure the new constant is defined near the top of the spec so both tests can
reference it and remove the local mutationLabels declarations from each test.In
@specs/governed-transitions/chunks/chunk-08-verification/context.md:
- Around line 57-83: Add an "Evidence" column to the verification checklist
table (the markdown table starting with "| Criterion | Pass? |") in context.md:
update the header row to include "| Evidence |", update the separator row
accordingly, and append an extra empty cell for Evidence to every existing
criterion row (e.g., F-1..F-9, REQ-1..REQ-10 and the other surface/check rows)
so each line becomes "| ... | | |" ready to record test names, screenshots,
logs, or file/line references.- Around line 40-47: The current verification text only looks for useMutation
calls but can miss other mutative paths; update the checks for journal.tsx and
machine.tsx to require that no mutative actions are reachable from these routes
(not just absence of useMutation), e.g., explicitly verify there are no
mutation-related hooks or wrapped hooks, no direct action calls that perform
state/server mutations, and that N8nWorkflowBlock is rendered with
readOnly={true} (for machine.tsx) and any interactive handlers (drag/drop,
add/edit node/edge, save actions) are disabled or absent; keep useMutation as
one concrete check but broaden wording to include these other mutative entry
points and to assert handlers/props are set to read-only/no-op.</details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro **Run ID**: `b2e8f31b-14f4-4bd2-8a95-b94f55039215` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 064f933564ecfafc8a74961f2aaebe9b179459a2 and 99e9a02b4fb2c8cbfeb2f88d8f958c84facf19fd. </details> <details> <summary>⛔ Files ignored due to path filters (2)</summary> * `bun.lock` is excluded by `!**/*.lock` * `convex/_generated/api.d.ts` is excluded by `!**/_generated/**` </details> <details> <summary>📒 Files selected for processing (40)</summary> * `convex/demo/fluentConvex.ts` * `convex/demo/governedTransitions.ts` * `convex/demo/governedTransitionsEffects.ts` * `convex/demo/machines/__tests__/loanApplication.test.ts` * `convex/demo/machines/loanApplication.machine.ts` * `convex/demo/machines/registry.ts` * `convex/schema.ts` * `e2e/governed-transitions.spec.ts` * `package.json` * `specs/governed-transitions/PRD.md` * `specs/governed-transitions/chunks/chunk-01-schema-machine/context.md` * `specs/governed-transitions/chunks/chunk-01-schema-machine/tasks.md` * `specs/governed-transitions/chunks/chunk-02-backend-engine/context.md` * `specs/governed-transitions/chunks/chunk-02-backend-engine/tasks.md` * `specs/governed-transitions/chunks/chunk-03-backend-queries/context.md` * `specs/governed-transitions/chunks/chunk-03-backend-queries/tasks.md` * `specs/governed-transitions/chunks/chunk-04-frontend-routes/context.md` * `specs/governed-transitions/chunks/chunk-04-frontend-routes/tasks.md` * `specs/governed-transitions/chunks/chunk-05-frontend-adapters/context.md` * `specs/governed-transitions/chunks/chunk-05-frontend-adapters/tasks.md` * `specs/governed-transitions/chunks/chunk-06-e2e-core/context.md` * `specs/governed-transitions/chunks/chunk-06-e2e-core/tasks.md` * `specs/governed-transitions/chunks/chunk-07-e2e-observer/context.md` * `specs/governed-transitions/chunks/chunk-07-e2e-observer/tasks.md` * `specs/governed-transitions/chunks/chunk-08-verification/context.md` * `specs/governed-transitions/chunks/chunk-08-verification/tasks.md` * `specs/governed-transitions/chunks/manifest.md` * `specs/governed-transitions/design.md` * `specs/governed-transitions/gap-analysis.md` * `specs/governed-transitions/tasks.md` * `src/components/header.tsx` * `src/components/ui/interactive-logs-table-shadcnui.tsx` * `src/components/ui/n8n-workflow-block-shadcnui.tsx` * `src/routeTree.gen.ts` * `src/routes/demo/governed-transitions/_components/GovernedTransitionsJournalView.tsx` * `src/routes/demo/governed-transitions/_components/GovernedTransitionsMachineView.tsx` * `src/routes/demo/governed-transitions/index.tsx` * `src/routes/demo/governed-transitions/journal.tsx` * `src/routes/demo/governed-transitions/machine.tsx` * `src/routes/demo/governed-transitions/route.tsx` </details> <details> <summary>✅ Files skipped from review due to trivial changes (1)</summary> * src/routeTree.gen.ts </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (20)</summary> * src/routes/demo/governed-transitions/machine.tsx * package.json * convex/demo/fluentConvex.ts * specs/governed-transitions/chunks/chunk-08-verification/tasks.md * specs/governed-transitions/chunks/chunk-01-schema-machine/tasks.md * convex/schema.ts * specs/governed-transitions/chunks/chunk-06-e2e-core/context.md * specs/governed-transitions/chunks/chunk-06-e2e-core/tasks.md * convex/demo/machines/loanApplication.machine.ts * src/routes/demo/governed-transitions/route.tsx * src/components/header.tsx * convex/demo/machines/registry.ts * specs/governed-transitions/chunks/chunk-05-frontend-adapters/tasks.md * specs/governed-transitions/chunks/chunk-07-e2e-observer/tasks.md * specs/governed-transitions/chunks/chunk-04-frontend-routes/tasks.md * specs/governed-transitions/gap-analysis.md * specs/governed-transitions/chunks/chunk-03-backend-queries/tasks.md * specs/governed-transitions/chunks/manifest.md * convex/demo/machines/__tests__/loanApplication.test.ts * src/routes/demo/governed-transitions/index.tsx </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Connorbelez
added a commit
that referenced
this pull request
Mar 13, 2026
- Hash-chain rejected transitions for complete audit evidence - Derive event types from machine config instead of hardcoding - Validate payload is plain object before spreading into XState event - Add take(200) limits and index-ordered queries to prevent unbounded reads - Add by_created index on demo_gt_entities for ordered listing - Trim whitespace in hasCompleteData guard + add POC TODO comment - Validate loan amount is finite & positive before mutation - Wrap Seed/Lifecycle buttons in try/catch error handling - Derive journal stats from filtered data so counts match visible rows - Add aria-label/expanded/controls to filter toggle for accessibility - Guard editing when external props are supplied (half-controlled fix) - Handle empty node arrays in Math.max content size init - Use more specific regex assertion in e2e test - Align design.md guard description and IP field with implementation - Sync chunk task completion status and fix e2e test path in specs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (2)
convex/demo/governedTransitions.ts (1)
492-507: Avoid full-table scans for stats counts.
collect()on the whole journal table will become expensive as data grows. Prefer indexed/aggregated counting for this endpoint.As per coding guidelines: “Use convex-dev-aggregate component whenever working on efficient counts, sums, rankings, leaderboards, or aggregate analytics over large datasets.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@convex/demo/governedTransitions.ts` around lines 492 - 507, The current code does a full-table scan by calling collect() on "demo_gt_journal" and iterating entries to compute transitioned/rejected counts; replace this with aggregated/indexed counts using the convex-dev-aggregate component: call the aggregate/count helper to get total and transitioned counts (e.g., a count for outcome === "transitioned" and a total count on "demo_gt_journal") and compute rejected = total - transitioned, then return { total, transitioned, rejected }; update/remove the entries, transitioned, and rejected iteration logic so you no longer load the whole table into memory.src/components/ui/interactive-logs-table-shadcnui.tsx (1)
28-32: Prefer interfaces for these object shapes.
LogandInteractiveLogsTablePropsalready follow the repo convention, butFiltersplus the inline prop object types switch back to type literals. Converting those shapes to named interfaces would keep this file consistent and easier to reuse. As per coding guidelines,Use interface for defining object shapes in TypeScript files within TanStack Router codebase.Also applies to: 132-140, 253-261
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/interactive-logs-table-shadcnui.tsx` around lines 28 - 32, The file mixes type aliases and interfaces; convert the object-shaped type aliases to interfaces to match repo convention: replace the Filters type alias with an interface named Filters and convert any inline prop object types (e.g., the object shapes around lines referenced in the comment — the prop type for InteractiveLogsTableProps and the Log-like inline shapes at the other spots) into named interfaces (e.g., InteractiveLogsTableProps and Log are already interfaces so make the other inline shapes interfaces too) and update all references to use the new interface names; ensure exported names and usages (Filters, any inline prop objects) are updated accordingly without changing runtime behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@convex/demo/governedTransitions.ts`:
- Around line 455-464: The current query on demo_gt_journal applies .take(200)
before filtering by outcome so matching older rows can be dropped; change the
query to filter by outcome in-index (use a composite index) instead of
post-filtering: add a composite index (e.g. "by_entity_outcome") on fields
entityId and outcome in the table schema, then update the query to use
ctx.db.query("demo_gt_journal").withIndex("by_entity_outcome", q =>
q.eq("entityId", entityId).eq("outcome", outcome)).order("desc").take(200) so
the outcome filter is applied by the index before taking results. Ensure you
remove the post-take .filter((e) => e.outcome === outcome) and reference
demo_gt_journal, withIndex, entityId, outcome, and take(200) when making the
change.
In `@specs/governed-transitions/chunks/chunk-03-backend-queries/context.md`:
- Around line 11-21: The examples still use full-table collects and in-memory
sorting (listEntities uses .order("desc") and .collect(), and the fallback path
in getJournal does the same); update both to the bounded/indexed pattern by
querying the demo_gt_entities table through a specific index and applying a
range/limit (or cursor/pagination token) instead of collecting the entire table,
and update handlers to accept pagination args (limit/cursor) and use
index-scoped methods (e.g., index-name + range/limit or cursor-based collect) so
reads are bounded and ordered by the backend index rather than in-memory
sorting.
- Around line 89-97: Replace the hardcoded allEvents array with event types
derived from the machine definition: compute the list of event type strings from
machineDef (e.g., gather keys from machineDef.events or from the transition/on
maps in machineDef.config) and use that list in the filter that calls
machineDef.transition(restoredState, { type: eventType }); keep references to
restoredState and entity.status unchanged so the filter still compares nextValue
!== entity.status.
- Around line 336-340: Update the "Quality Gate" section so completion requires
running and passing `bun check`, `bun typecheck`, and `bunx convex codegen`;
modify the command list under the Quality Gate heading (the lines currently
listing `bun check` and `bun typecheck`) to also include `bunx convex codegen`
and update the summary sentence to state that all three commands must pass
before tasks are considered complete.
In `@specs/governed-transitions/chunks/chunk-04-frontend-routes/context.md`:
- Line 173: Update the stats-source note in the markdown to say the three stat
cards derive their counts from the filtered getJournal result (respecting active
UI filters) instead of from getJournalStats; replace the reference to
getJournalStats with getJournal and explicitly mention that journal.tsx computes
counts from the filtered getJournal output so the cards reflect active filters.
In `@specs/governed-transitions/design.md`:
- Around line 349-376: Update this markdown snippet to match current
implementation: instead of spreading raw payload into the event, show that
payload is sanitized (e.g., build a sanitizedPayload and use that when
constructing event passed to machineDef.transition), and when a transition is
rejected still record the journal entry and schedule the hash-chain handoff
(mirror the behavior in convex/demo/governedTransitions.ts) — reference the
transition call (machineDef.transition), the journal insert (ctx.db.insert into
demo_gt_journal), and the hash-chain handoff scheduling logic so the example
reflects sanitization and scheduling for rejected outcomes as in the real code.
- Around line 561-564: The API table in the design doc is out of sync with the
implementation: update the signatures for getJournal to use the narrowed outcome
union type the code expects (replace outcome?: v.string() with the precise union
used in the implementation), and update getValidTransitions to return an array
of objects with { eventType, targetState } instead of string[]; also verify
getJournalStats and getEffectsLog return types match the code and adjust their
documented shapes (e.g., Doc<"demo_gt_journal">[], { total:number,
transitioned:number, rejected:number }, Doc<"demo_gt_effects_log">[]) so the
table accurately reflects the runtime API (refer to getJournal,
getValidTransitions, getJournalStats, and getEffectsLog to mirror the exact
types and field names).
- Around line 637-639: The design doc's "Top bar" description is out of sync: it
claims stats come from getJournalStats but the Journal page actually derives
counts from the filtered getJournal results to reflect active filters; update
the text to state that the three stat cards (Total, Transitioned, Rejected) are
computed from the filtered getJournal response (and note the Entity dropdown and
Outcome toggle affect these counts) and remove or deprecate the getJournalStats
reference so the spec matches the implementation.
In `@specs/governed-transitions/tasks.md`:
- Around line 23-27: The tasks.md descriptions for T-016 and T-019 are out of
date: update T-016 to reflect that executeEffect (internalMutation) currently
inserts a demo_gt_effects_log entry with status "completed" directly (instead of
inserting "scheduled" then updating to "completed"), and update T-019 to state
that the loanApplication machine's guard validates required fields on
context.data (not the incoming event payload) when describing the
hasCompleteData check; reference the function/feature names executeEffect and
hasCompleteData (and the test file
convex/demo/machines/__tests__/loanApplication.test.ts) so the markdown matches
the actual implementation.
- Line 33: T-032 is out of date: the spec still says the Journal stats come from
getJournalStats but the implementation derives counts from the filtered
getJournal results to match visible rows; update the task text in the
specs/governed-transitions/tasks.md for T-032 to remove or replace the
getJournalStats reference and explicitly state that the top stats are computed
from the filtered getJournal data (and mention JournalViewer, getJournal, and
listEntities so reviewers can locate the related implementation).
In `@src/components/ui/interactive-logs-table-shadcnui.tsx`:
- Around line 508-517: The staggered enter animation uses delay: index * 0.02
which grows unbounded for large filteredLogs; change the delay calculation in
the motion.div within filteredLogs.map to clamp the delay (e.g., use
Math.min(index * 0.02, MAX_DELAY) or only apply index-based staggering for the
first N items) so later rows don’t wait excessively; update the transition.delay
expression accordingly and choose a sensible MAX_DELAY (or N) to keep the
animation snappy for large result sets.
- Around line 149-205: The toggle button (motion.button with onToggle) should be
made keyboard/form-safe and announce its state: add type="button" and
aria-expanded={expanded} to the motion.button, and wire aria-controls to the
details panel by giving the details motion.div a stable id (e.g.
`details-${log.id}` or similar unique identifier) and using that id in
aria-controls; ensure the details panel element that is conditionally rendered
(the motion.div with key="details") receives the matching id so screen readers
can correlate the control and the region.
---
Nitpick comments:
In `@convex/demo/governedTransitions.ts`:
- Around line 492-507: The current code does a full-table scan by calling
collect() on "demo_gt_journal" and iterating entries to compute
transitioned/rejected counts; replace this with aggregated/indexed counts using
the convex-dev-aggregate component: call the aggregate/count helper to get total
and transitioned counts (e.g., a count for outcome === "transitioned" and a
total count on "demo_gt_journal") and compute rejected = total - transitioned,
then return { total, transitioned, rejected }; update/remove the entries,
transitioned, and rejected iteration logic so you no longer load the whole table
into memory.
In `@src/components/ui/interactive-logs-table-shadcnui.tsx`:
- Around line 28-32: The file mixes type aliases and interfaces; convert the
object-shaped type aliases to interfaces to match repo convention: replace the
Filters type alias with an interface named Filters and convert any inline prop
object types (e.g., the object shapes around lines referenced in the comment —
the prop type for InteractiveLogsTableProps and the Log-like inline shapes at
the other spots) into named interfaces (e.g., InteractiveLogsTableProps and Log
are already interfaces so make the other inline shapes interfaces too) and
update all references to use the new interface names; ensure exported names and
usages (Filters, any inline prop objects) are updated accordingly without
changing runtime behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9a1fe061-e704-46e6-bc37-22e3c20cd30d
⛔ Files ignored due to path filters (1)
convex/_generated/api.d.tsis excluded by!**/_generated/**
📒 Files selected for processing (14)
convex/demo/governedTransitions.tsconvex/demo/machines/loanApplication.machine.tsconvex/schema.tse2e/governed-transitions.spec.tsspecs/governed-transitions/chunks/chunk-02-backend-engine/tasks.mdspecs/governed-transitions/chunks/chunk-03-backend-queries/context.mdspecs/governed-transitions/chunks/chunk-04-frontend-routes/context.mdspecs/governed-transitions/design.mdspecs/governed-transitions/tasks.mdsrc/components/ui/interactive-logs-table-shadcnui.tsxsrc/components/ui/n8n-workflow-block-shadcnui.tsxsrc/routes/demo/governed-transitions/_components/GovernedTransitionsJournalView.tsxsrc/routes/demo/governed-transitions/index.tsxsrc/routes/demo/governed-transitions/journal.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- e2e/governed-transitions.spec.ts
- convex/demo/machines/loanApplication.machine.ts
- src/routes/demo/governed-transitions/index.tsx
Merged
- Hash-chain rejected transitions for complete audit evidence - Derive event types from machine config instead of hardcoding - Validate payload is plain object before spreading into XState event - Add take(200) limits and index-ordered queries to prevent unbounded reads - Add by_created index on demo_gt_entities for ordered listing - Trim whitespace in hasCompleteData guard + add POC TODO comment - Validate loan amount is finite & positive before mutation - Wrap Seed/Lifecycle buttons in try/catch error handling - Derive journal stats from filtered data so counts match visible rows - Add aria-label/expanded/controls to filter toggle for accessibility - Guard editing when external props are supplied (half-controlled fix) - Handle empty node arrays in Math.max content size init - Use more specific regex assertion in e2e test - Align design.md guard description and IP field with implementation - Sync chunk task completion status and fix e2e test path in specs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Connorbelez
force-pushed
the
03-12-governed_transitions
branch
from
85a5ea6 to
b1227b8
Compare
Connorbelez
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (10)
specs/governed-transitions/chunks/chunk-01-schema-machine/context.md (1)
45-50:⚠️ Potential issue | 🟡 Minor
demo_gt_journal.sourcesnippet is missingip.The documented schema block omits
source.ip, but the implemented schema persists optionalip. Please align this snippet with runtime shape.Based on learnings: “Do not flag Markdown (.md) files for linting/formatting issues ... Only raise findings on .md files when they reveal architectural drift or implementation inconsistencies relative to the codebase.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@specs/governed-transitions/chunks/chunk-01-schema-machine/context.md` around lines 45 - 50, The documented schema snippet for demo_gt_journal.source is missing the optional ip field; update the v.object literal shown (the source: v.object({...}) block) to include ip as an optional string (e.g., ip: v.optional(v.string())) so the Markdown aligns with the runtime schema that persists source.ip.specs/governed-transitions/chunks/chunk-03-backend-queries/context.md (3)
338-341:⚠️ Potential issue | 🟡 MinorAdd
bunx convex codegento the quality gate.The completion checklist is missing Convex codegen refresh, which is required for generated API/type consistency after schema/backend changes.
Based on learnings: “Ensure
bun check,bun typecheck, andbunx convex codegenpass before considering tasks completed.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@specs/governed-transitions/chunks/chunk-03-backend-queries/context.md` around lines 338 - 341, Update the completion checklist to include running the Convex codegen command: after "bun check" and "bun typecheck" add "bunx convex codegen" so generated API/types are refreshed; ensure the instructions state to run "bunx convex codegen" and fix any issues it reports before marking tasks complete (refer to the existing checklist entries "bun check" and "bun typecheck" to locate where to add the new step).
11-21:⚠️ Potential issue | 🟡 MinorQuery examples still document unbounded collect/sort patterns.
These snippets should reflect the bounded/indexed read approach used in the backend (
withIndex+ limit/take), not full-table collect and in-memory ordering guidance.Based on learnings: “Do not flag Markdown (.md) files for linting/formatting issues ... Only raise findings on .md files when they reveal architectural drift or implementation inconsistencies relative to the codebase.”
Also applies to: 35-48
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@specs/governed-transitions/chunks/chunk-03-backend-queries/context.md` around lines 11 - 21, The example query listEntities currently uses an unbounded ctx.db.query("demo_gt_entities").order("desc").collect() pattern; change it to demonstrate the bounded/indexed read pattern used in the backend by calling the indexed helper (withIndex) on the query and applying a limit/take (e.g., take or limit parameter) rather than collect(), and preserve the sort direction via the index; update the listEntities example (and the other similar examples referred to around lines 35-48) to show ctx.db.query("demo_gt_entities").withIndex(...).take(...)/limit(...) instead of collect() to reflect the real implementation approach.
89-97:⚠️ Potential issue | 🟡 Minor
getValidTransitionsexample should not hardcode event arrays.Hardcoding event names in docs will drift; derive event types from the machine definition so docs match the implementation strategy.
Based on learnings: “Do not flag Markdown (.md) files for linting/formatting issues ... Only raise findings on .md files when they reveal architectural drift or implementation inconsistencies relative to the codebase.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@specs/governed-transitions/chunks/chunk-03-backend-queries/context.md` around lines 89 - 97, The example in getValidTransitions currently hardcodes allEvents; instead derive the candidate event types from the machine definition (use machineDef to enumerate declared events/transitions) so docs reflect the real machine. Update the logic that builds allEvents to inspect machineDef (e.g., the machineDef.events or the transitions map on machineDef) and produce the array used in the filter, keeping the rest of the filtering logic that calls machineDef.transition(restoredState, { type: eventType }) and compares next.value to entity.status.specs/governed-transitions/design.md (3)
100-105:⚠️ Potential issue | 🟡 Minor
source.ipis missing from two contract snippets.Both the schema snippet and transition-args snippet omit
source.ip, but the runtime schema includes it. This creates contract drift around audit metadata.🛠️ Suggested doc patch
source: v.object({ channel: v.string(), actorId: v.optional(v.string()), actorType: v.optional(v.string()), sessionId: v.optional(v.string()), + ip: v.optional(v.string()), }),Based on learnings: “Do not flag Markdown (.md) files for linting/formatting issues ... Only raise findings on .md files when they reveal architectural drift or implementation inconsistencies relative to the codebase.”
Also applies to: 317-322
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@specs/governed-transitions/design.md` around lines 100 - 105, The docs omit the source.ip field causing drift vs runtime: update the v.object schema snippet (the source v.object({...}) block) and the corresponding transition-args snippet to include source.ip (e.g., add a line for ip: v.optional(v.string()) or the appropriate type used at runtime), ensuring the schema in the docs matches the runtime audit metadata shape referenced elsewhere.
561-569:⚠️ Potential issue | 🟡 MinorAPI table/docs are stale for
getJournalandgetValidTransitions.The docs still describe a broad
outcome?: v.string()andstring[]transitions, which no longer match the implemented query contracts and return shape.Based on learnings: “Do not flag Markdown (.md) files for linting/formatting issues ... Only raise findings on .md files when they reveal architectural drift or implementation inconsistencies relative to the codebase.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@specs/governed-transitions/design.md` around lines 561 - 569, The API docs for getJournal and getValidTransitions are out of sync with the code: update the table and the implementation note so the parameter schemas and return types match the actual query contracts (e.g., replace the broad outcome?: v.string() with the actual outcome filter schema used by the query and change the getValidTransitions return type from generic string[] to the real shape returned by the service), and adjust the getValidTransitions implementation notes to exactly describe the current algorithm (how you hydrate state and invoke machineDef.transition and how you determine/format valid transitions). Reference the symbols getJournal and getValidTransitions in the docs to ensure the parameter and return schemas reflect the implemented types and filters.
308-310:⚠️ Potential issue | 🟡 MinorFix
_generatedimport paths in the backend snippet.For
convex/demo/governedTransitions.ts,./_generated/*resolves to the wrong directory. Use../_generated/*to match the actual file location.🛠️ Suggested doc patch
-import { mutation, query, internalMutation } from "./_generated/server"; -import { internal } from "./_generated/api"; +import { mutation, query, internalMutation } from "../_generated/server"; +import { internal } from "../_generated/api";Based on learnings: “Do not flag Markdown (.md) files for linting/formatting issues ... Only raise findings on .md files when they reveal architectural drift or implementation inconsistencies relative to the codebase.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@specs/governed-transitions/design.md` around lines 308 - 310, The backend snippet's imports (e.g., the identifiers mutation, query, internalMutation and internal, and machineRegistry/EntityType) reference "./_generated/*" which resolves incorrectly; update those import specifiers to reference the parent directory (use "../_generated/*") so the imports point to the actual generated module locations (change the three imports that currently use "./_generated/..." to "../_generated/...").src/components/ui/interactive-logs-table-shadcnui.tsx (2)
514-517:⚠️ Potential issue | 🟠 MajorCap stagger delay to avoid slow “half-rendered” lists on bigger result sets.
delay: index * 0.02grows unbounded and can push late rows out by seconds.⚡ Suggested patch
transition={{ duration: 0.2, - delay: index * 0.02, + delay: Math.min(index, 8) * 0.02, }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/interactive-logs-table-shadcnui.tsx` around lines 514 - 517, The staggered animation delay uses an unbounded expression (delay: index * 0.02) which can make late rows appear very slowly; update the delay calculation in the component (the transition prop where delay is set) to cap the value (e.g., clamp or Math.min) so the maximum delay is limited (choose a cap like 0.2–0.5s) while keeping the existing index-based staggering; modify the delay assignment near the transition block in interactive-logs-table-shadcnui.tsx to use the capped expression instead of index * 0.02.
149-205:⚠️ Potential issue | 🟡 MinorExpose disclosure state on each expandable row control.
The row toggle should declare button semantics and expanded/collapsed state for assistive tech, and reference its details panel.
♿ Suggested patch
<motion.button + type="button" onClick={onToggle} + aria-expanded={expanded} + aria-controls={`log-details-${log.id}`} className="w-full p-4 text-left transition-colors hover:bg-muted/50 active:bg-muted/70" whileHover={{ backgroundColor: "rgba(0,0,0,0.02)" }} > @@ {expanded && ( <motion.div + id={`log-details-${log.id}`} key="details"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/interactive-logs-table-shadcnui.tsx` around lines 149 - 205, The toggle button (motion.button using onToggle and expanded) lacks accessible disclosure semantics; update the button to include aria-expanded={expanded} and aria-controls pointing to the details panel, and ensure the details panel (the motion.div rendered when expanded, key="details") has a stable id that matches aria-controls and a complementary role (e.g., role="region") and aria-labelledby if needed; keep using the existing onToggle/expanded logic and generate a deterministic id (e.g., based on log.id or index) so each row's button and motion.div are linked for assistive tech.convex/demo/governedTransitions.ts (1)
461-469:⚠️ Potential issue | 🟠 MajorOutcome filtering after
take(200)can silently drop matching rows.At Line 468-469, filtering happens after limiting results, so older matching entries may be omitted.
💡 Proposed fix (query in-index before take)
- if (entityId && outcome) { - return ( - await ctx.db - .query("demo_gt_journal") - .withIndex("by_entity", (q) => q.eq("entityId", entityId)) - .order("desc") - .take(200) - ).filter((e) => e.outcome === outcome); - } + if (entityId && outcome) { + return await ctx.db + .query("demo_gt_journal") + .withIndex("by_entity_outcome_time", (q) => + q.eq("entityId", entityId).eq("outcome", outcome) + ) + .order("desc") + .take(200); + }# convex/schema.ts (demo_gt_journal indices) - .index("by_entity", ["entityId", "timestamp"]) + .index("by_entity", ["entityId", "timestamp"]) + .index("by_entity_outcome_time", ["entityId", "outcome", "timestamp"])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@convex/demo/governedTransitions.ts` around lines 461 - 469, The current query uses withIndex("by_entity", q => q.eq("entityId", entityId)).take(200) and then filters by outcome, which can drop matches because filtering occurs after limiting; change the query to perform the outcome filter in-index before take by using a composite index (e.g., withIndex("by_entity_outcome", q => q.eq("entityId", entityId).eq("outcome", outcome))) against "demo_gt_journal" and remove the post-take .filter so the client-side limit doesn't exclude matching rows; if the composite index doesn't exist, add it to the schema before making this change.
🧹 Nitpick comments (5)
convex/demo/governedTransitionsEffects.ts (1)
10-64: Consider extracting shared handler logic.All four effect handlers (
notifyReviewer,notifyApplicant,scheduleFunding,generateDocuments) have identical implementations. For a POC/demo this is acceptable since each may diverge in production, but you could reduce duplication with a factory function:♻️ Optional refactor
const createEffectHandler = () => internalMutation({ args: effectArgs, handler: async (ctx, { entityId, journalEntryId, effectName }) => { await ctx.db.insert("demo_gt_effects_log", { entityId, journalEntryId, effectName, status: "completed", scheduledAt: Date.now(), completedAt: Date.now(), }); }, }); export const notifyReviewer = createEffectHandler(); export const notifyApplicant = createEffectHandler(); export const scheduleFunding = createEffectHandler(); export const generateDocuments = createEffectHandler();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@convex/demo/governedTransitionsEffects.ts` around lines 10 - 64, All four internalMutations (notifyReviewer, notifyApplicant, scheduleFunding, generateDocuments) duplicate the same handler logic; extract the shared implementation into a factory function (e.g., createEffectHandler) that returns internalMutation({ args: effectArgs, handler: async (ctx, { entityId, journalEntryId, effectName }) => { await ctx.db.insert("demo_gt_effects_log", { entityId, journalEntryId, effectName, status: "completed", scheduledAt: Date.now(), completedAt: Date.now(), }); } }) and then replace each exported constant with a call to createEffectHandler() so the duplicated handler code is centralized while keeping the exported names unchanged.e2e/governed-transitions.spec.ts (1)
60-74: Consider making the entity selector more robust.The current locator chain (
page.locator("div").filter({ hasText: label }).getByRole("button").first()) may be fragile if the DOM structure changes. Consider adding adata-testidattribute to entity cards for more stable selection.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/governed-transitions.spec.ts` around lines 60 - 74, The selector in selectEntity is fragile: entityCard is found via page.locator("div").filter({ hasText: label }).getByRole("button").first(), which will break if DOM changes; update the app to add a stable data attribute (e.g., data-testid="entity-card") on the entity card and change selectEntity to locate by that test id combined with the label (or use getByTestId/getByRole with the test id) so the test targets the element via the new data-testid instead of the brittle div+text filter.convex/demo/governedTransitions.ts (3)
785-804: Actor/type journal queries return unbounded result sets.At Line 792 and Line 803,
.collect()can return very large payloads. Add ordering + bounded page size (and cursor pagination) to avoid oversized responses.💡 Proposed bounded query pattern
return await ctx.db .query("demo_gt_journal") .withIndex("by_actor", (q) => q.eq("source.actorId", actorId)) - .collect(); + .order("desc") + .take(200);return await ctx.db .query("demo_gt_journal") .withIndex("by_type_and_time", (q) => q.eq("entityType", entityType)) - .collect(); + .order("desc") + .take(200);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@convex/demo/governedTransitions.ts` around lines 785 - 804, The queries getJournalByActor and getJournalByEntityType currently call .collect() and can return unbounded results; change both handlers to accept pagination inputs (e.g., optional cursor and limit/pageSize), apply a deterministic sort on a time or monotonic field (e.g., the journal entry timestamp or _id) and use .limit(pageSize) plus cursor-aware fetching (return items plus nextCursor) instead of .collect(), so the queries use the existing indexes (withIndex("by_actor") / withIndex("by_type_and_time")), .sort(...) and .limit(...) to bound responses and implement cursor pagination in the handler and response.
498-513:getJournalStatsdoes an unbounded full-table scan.At Line 498,
.collect()over all journal rows will degrade as data grows. Consider storing rolling counters (or pre-aggregates) updated on write to keep this query O(1).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@convex/demo/governedTransitions.ts` around lines 498 - 513, getJournalStats performs an unbounded full-table scan of demo_gt_journal by calling .collect(), which will not scale; change the design to read from a pre-aggregated counter instead (e.g., a new demo_gt_journal_stats record) and update that counter atomically on every write to the journal. Modify the write paths that insert or update journal rows (e.g., the functions that create entries or mark outcomes) to increment/decrement the appropriate fields (total, transitioned, rejected) in the stats record within the same transaction, and change getJournalStats to fetch and return the single stats record (O(1)) rather than scanning demo_gt_journal. Ensure updates are atomic and initialize the stats record if missing.
289-293: Existence check can avoid scanning all entities.At Line 290, using
.collect()just to test non-empty is more expensive than needed.💡 Proposed lightweight existence check
- const existing = await ctx.db.query("demo_gt_entities").collect(); - if (existing.length > 0) { + const existing = await ctx.db + .query("demo_gt_entities") + .withIndex("by_created") + .take(1); + if (existing.length > 0) { return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@convex/demo/governedTransitions.ts` around lines 289 - 293, The existence check currently calls ctx.db.query("demo_gt_entities").collect() and tests existing.length > 0 which unnecessarily retrieves all rows; replace this with a lightweight single-row check such as using ctx.db.query("demo_gt_entities").first() (or an equivalent .take(1) API) and test the returned value for null/undefined instead of collecting all entities, updating the code around the existing variable and the conditional that returns early.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@convex/demo/governedTransitions.ts`:
- Around line 227-249: The public transition mutation (transition.handler)
currently accepts client-supplied source metadata and forwards it to
executeTransition; call getAuth() inside the handler to obtain the server-side
authenticated session (sessionId and actor identity) and either validate the
incoming source against it or replace the client-supplied
actorId/actorType/sessionId with the values from getAuth(), then pass the
sanitized source to executeTransition; ensure this change is made in the
transition mutation handler (not in executeTransition) so the .public() endpoint
cannot persist spoofed audit metadata.
In `@specs/governed-transitions/chunks/chunk-03-backend-queries/tasks.md`:
- Line 7: The task description for T-019 is inconsistent with the
implementation: the hasCompleteData guard on the loanApplication machine checks
context.data (hydrated entity) rather than the event payload and SUBMIT has no
payload; update the task text to match the code or change the guard accordingly.
Either (A) change the task wording to say "hasCompleteData reads from
context.data (entity) and should be tested by hydrating resolveState with
context.data before calling transition" and mention testing SUBMIT with no
payload, or (B) modify the machine so hasCompleteData reads required fields from
the event payload on SUBMIT; reference the loanApplication machine and the
hasCompleteData guard and the SUBMIT event when making the change. Ensure tests
use the resolveState + transition pattern described.
In `@specs/governed-transitions/chunks/chunk-04-frontend-routes/context.md`:
- Around line 189-193: The code still calls
api.demo.governedTransitions.getJournalStats even though the Journal Viewer
derives stats from getJournal; remove the getJournalStats useQuery call (or
replace its Usage) so the example matches implementation: keep
useQuery(api.demo.governedTransitions.getJournal, filterArgs) and use that data
to compute stats, and delete or comment out the line referencing
api.demo.governedTransitions.getJournalStats and any consumers of that variable
(e.g., the stats variable) so the example reflects the current Journal Viewer
behavior.
In `@specs/governed-transitions/PRD.md`:
- Around line 150-158: The Out of Scope text is inaccurate because the demo
guard checks context.data rather than the event payload; update the PRD wording
to reflect that hasCompleteData (from loanApplication.machine.ts) reads
machineContext.context.data (hydrated at creation) and that the SUBMIT event
carries no payload in the demo, or alternatively change the demo guard to
inspect the event payload; reference the hasCompleteData guard and SUBMIT event
when updating the description so it matches the implementation.
---
Duplicate comments:
In `@convex/demo/governedTransitions.ts`:
- Around line 461-469: The current query uses withIndex("by_entity", q =>
q.eq("entityId", entityId)).take(200) and then filters by outcome, which can
drop matches because filtering occurs after limiting; change the query to
perform the outcome filter in-index before take by using a composite index
(e.g., withIndex("by_entity_outcome", q => q.eq("entityId",
entityId).eq("outcome", outcome))) against "demo_gt_journal" and remove the
post-take .filter so the client-side limit doesn't exclude matching rows; if the
composite index doesn't exist, add it to the schema before making this change.
In `@specs/governed-transitions/chunks/chunk-01-schema-machine/context.md`:
- Around line 45-50: The documented schema snippet for demo_gt_journal.source is
missing the optional ip field; update the v.object literal shown (the source:
v.object({...}) block) to include ip as an optional string (e.g., ip:
v.optional(v.string())) so the Markdown aligns with the runtime schema that
persists source.ip.
In `@specs/governed-transitions/chunks/chunk-03-backend-queries/context.md`:
- Around line 338-341: Update the completion checklist to include running the
Convex codegen command: after "bun check" and "bun typecheck" add "bunx convex
codegen" so generated API/types are refreshed; ensure the instructions state to
run "bunx convex codegen" and fix any issues it reports before marking tasks
complete (refer to the existing checklist entries "bun check" and "bun
typecheck" to locate where to add the new step).
- Around line 11-21: The example query listEntities currently uses an unbounded
ctx.db.query("demo_gt_entities").order("desc").collect() pattern; change it to
demonstrate the bounded/indexed read pattern used in the backend by calling the
indexed helper (withIndex) on the query and applying a limit/take (e.g., take or
limit parameter) rather than collect(), and preserve the sort direction via the
index; update the listEntities example (and the other similar examples referred
to around lines 35-48) to show
ctx.db.query("demo_gt_entities").withIndex(...).take(...)/limit(...) instead of
collect() to reflect the real implementation approach.
- Around line 89-97: The example in getValidTransitions currently hardcodes
allEvents; instead derive the candidate event types from the machine definition
(use machineDef to enumerate declared events/transitions) so docs reflect the
real machine. Update the logic that builds allEvents to inspect machineDef
(e.g., the machineDef.events or the transitions map on machineDef) and produce
the array used in the filter, keeping the rest of the filtering logic that calls
machineDef.transition(restoredState, { type: eventType }) and compares
next.value to entity.status.
In `@specs/governed-transitions/design.md`:
- Around line 100-105: The docs omit the source.ip field causing drift vs
runtime: update the v.object schema snippet (the source v.object({...}) block)
and the corresponding transition-args snippet to include source.ip (e.g., add a
line for ip: v.optional(v.string()) or the appropriate type used at runtime),
ensuring the schema in the docs matches the runtime audit metadata shape
referenced elsewhere.
- Around line 561-569: The API docs for getJournal and getValidTransitions are
out of sync with the code: update the table and the implementation note so the
parameter schemas and return types match the actual query contracts (e.g.,
replace the broad outcome?: v.string() with the actual outcome filter schema
used by the query and change the getValidTransitions return type from generic
string[] to the real shape returned by the service), and adjust the
getValidTransitions implementation notes to exactly describe the current
algorithm (how you hydrate state and invoke machineDef.transition and how you
determine/format valid transitions). Reference the symbols getJournal and
getValidTransitions in the docs to ensure the parameter and return schemas
reflect the implemented types and filters.
- Around line 308-310: The backend snippet's imports (e.g., the identifiers
mutation, query, internalMutation and internal, and machineRegistry/EntityType)
reference "./_generated/*" which resolves incorrectly; update those import
specifiers to reference the parent directory (use "../_generated/*") so the
imports point to the actual generated module locations (change the three imports
that currently use "./_generated/..." to "../_generated/...").
In `@src/components/ui/interactive-logs-table-shadcnui.tsx`:
- Around line 514-517: The staggered animation delay uses an unbounded
expression (delay: index * 0.02) which can make late rows appear very slowly;
update the delay calculation in the component (the transition prop where delay
is set) to cap the value (e.g., clamp or Math.min) so the maximum delay is
limited (choose a cap like 0.2–0.5s) while keeping the existing index-based
staggering; modify the delay assignment near the transition block in
interactive-logs-table-shadcnui.tsx to use the capped expression instead of
index * 0.02.
- Around line 149-205: The toggle button (motion.button using onToggle and
expanded) lacks accessible disclosure semantics; update the button to include
aria-expanded={expanded} and aria-controls pointing to the details panel, and
ensure the details panel (the motion.div rendered when expanded, key="details")
has a stable id that matches aria-controls and a complementary role (e.g.,
role="region") and aria-labelledby if needed; keep using the existing
onToggle/expanded logic and generate a deterministic id (e.g., based on log.id
or index) so each row's button and motion.div are linked for assistive tech.
---
Nitpick comments:
In `@convex/demo/governedTransitions.ts`:
- Around line 785-804: The queries getJournalByActor and getJournalByEntityType
currently call .collect() and can return unbounded results; change both handlers
to accept pagination inputs (e.g., optional cursor and limit/pageSize), apply a
deterministic sort on a time or monotonic field (e.g., the journal entry
timestamp or _id) and use .limit(pageSize) plus cursor-aware fetching (return
items plus nextCursor) instead of .collect(), so the queries use the existing
indexes (withIndex("by_actor") / withIndex("by_type_and_time")), .sort(...) and
.limit(...) to bound responses and implement cursor pagination in the handler
and response.
- Around line 498-513: getJournalStats performs an unbounded full-table scan of
demo_gt_journal by calling .collect(), which will not scale; change the design
to read from a pre-aggregated counter instead (e.g., a new demo_gt_journal_stats
record) and update that counter atomically on every write to the journal. Modify
the write paths that insert or update journal rows (e.g., the functions that
create entries or mark outcomes) to increment/decrement the appropriate fields
(total, transitioned, rejected) in the stats record within the same transaction,
and change getJournalStats to fetch and return the single stats record (O(1))
rather than scanning demo_gt_journal. Ensure updates are atomic and initialize
the stats record if missing.
- Around line 289-293: The existence check currently calls
ctx.db.query("demo_gt_entities").collect() and tests existing.length > 0 which
unnecessarily retrieves all rows; replace this with a lightweight single-row
check such as using ctx.db.query("demo_gt_entities").first() (or an equivalent
.take(1) API) and test the returned value for null/undefined instead of
collecting all entities, updating the code around the existing variable and the
conditional that returns early.
In `@convex/demo/governedTransitionsEffects.ts`:
- Around line 10-64: All four internalMutations (notifyReviewer,
notifyApplicant, scheduleFunding, generateDocuments) duplicate the same handler
logic; extract the shared implementation into a factory function (e.g.,
createEffectHandler) that returns internalMutation({ args: effectArgs, handler:
async (ctx, { entityId, journalEntryId, effectName }) => { await
ctx.db.insert("demo_gt_effects_log", { entityId, journalEntryId, effectName,
status: "completed", scheduledAt: Date.now(), completedAt: Date.now(), }); } })
and then replace each exported constant with a call to createEffectHandler() so
the duplicated handler code is centralized while keeping the exported names
unchanged.
In `@e2e/governed-transitions.spec.ts`:
- Around line 60-74: The selector in selectEntity is fragile: entityCard is
found via page.locator("div").filter({ hasText: label
}).getByRole("button").first(), which will break if DOM changes; update the app
to add a stable data attribute (e.g., data-testid="entity-card") on the entity
card and change selectEntity to locate by that test id combined with the label
(or use getByTestId/getByRole with the test id) so the test targets the element
via the new data-testid instead of the brittle div+text filter.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3ab8e96e-6777-4e03-a717-8101fa00f2df
⛔ Files ignored due to path filters (2)
bun.lockis excluded by!**/*.lockconvex/_generated/api.d.tsis excluded by!**/_generated/**
📒 Files selected for processing (39)
convex/demo/fluentConvex.tsconvex/demo/governedTransitions.tsconvex/demo/governedTransitionsEffects.tsconvex/demo/machines/__tests__/loanApplication.test.tsconvex/demo/machines/loanApplication.machine.tsconvex/demo/machines/registry.tsconvex/schema.tse2e/governed-transitions.spec.tsspecs/governed-transitions/PRD.mdspecs/governed-transitions/chunks/chunk-01-schema-machine/context.mdspecs/governed-transitions/chunks/chunk-01-schema-machine/tasks.mdspecs/governed-transitions/chunks/chunk-02-backend-engine/context.mdspecs/governed-transitions/chunks/chunk-02-backend-engine/tasks.mdspecs/governed-transitions/chunks/chunk-03-backend-queries/context.mdspecs/governed-transitions/chunks/chunk-03-backend-queries/tasks.mdspecs/governed-transitions/chunks/chunk-04-frontend-routes/context.mdspecs/governed-transitions/chunks/chunk-04-frontend-routes/tasks.mdspecs/governed-transitions/chunks/chunk-05-frontend-adapters/context.mdspecs/governed-transitions/chunks/chunk-05-frontend-adapters/tasks.mdspecs/governed-transitions/chunks/chunk-06-e2e-core/context.mdspecs/governed-transitions/chunks/chunk-06-e2e-core/tasks.mdspecs/governed-transitions/chunks/chunk-07-e2e-observer/context.mdspecs/governed-transitions/chunks/chunk-07-e2e-observer/tasks.mdspecs/governed-transitions/chunks/chunk-08-verification/context.mdspecs/governed-transitions/chunks/chunk-08-verification/tasks.mdspecs/governed-transitions/chunks/manifest.mdspecs/governed-transitions/design.mdspecs/governed-transitions/gap-analysis.mdspecs/governed-transitions/tasks.mdsrc/components/header.tsxsrc/components/ui/interactive-logs-table-shadcnui.tsxsrc/components/ui/n8n-workflow-block-shadcnui.tsxsrc/routeTree.gen.tssrc/routes/demo/governed-transitions/_components/GovernedTransitionsJournalView.tsxsrc/routes/demo/governed-transitions/_components/GovernedTransitionsMachineView.tsxsrc/routes/demo/governed-transitions/index.tsxsrc/routes/demo/governed-transitions/journal.tsxsrc/routes/demo/governed-transitions/machine.tsxsrc/routes/demo/governed-transitions/route.tsx
✅ Files skipped from review due to trivial changes (3)
- specs/governed-transitions/chunks/chunk-08-verification/context.md
- specs/governed-transitions/gap-analysis.md
- specs/governed-transitions/chunks/chunk-06-e2e-core/tasks.md
🚧 Files skipped from review as they are similar to previous changes (10)
- src/routes/demo/governed-transitions/machine.tsx
- convex/demo/fluentConvex.ts
- src/components/header.tsx
- specs/governed-transitions/chunks/chunk-08-verification/tasks.md
- specs/governed-transitions/chunks/chunk-07-e2e-observer/tasks.md
- convex/demo/machines/registry.ts
- src/routes/demo/governed-transitions/route.tsx
- convex/demo/machines/loanApplication.machine.ts
- specs/governed-transitions/chunks/chunk-05-frontend-adapters/tasks.md
- src/routes/demo/governed-transitions/_components/GovernedTransitionsJournalView.tsx
Connorbelez
added a commit
that referenced
this pull request
Mar 16, 2026
- Add daily reconciliation cron (06:00 UTC) that runs Layer 1 check - Create reconciliationAction.ts with internalQuery (no auth required) and internalAction for cron scheduling - Discrepancies logged as P0 errors via console.error - On-demand adminQuery reconciliation remains available Resolves SPEC Open Question #5: daily cron + on-demand admin query. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Connorbelez
added a commit
that referenced
this pull request
Mar 16, 2026
- Add daily reconciliation cron (06:00 UTC) that runs Layer 1 check - Create reconciliationAction.ts with internalQuery (no auth required) and internalAction for cron scheduling - Discrepancies logged as P0 errors via console.error - On-demand adminQuery reconciliation remains available Resolves SPEC Open Question #5: daily cron + on-demand admin query. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Connorbelez
added a commit
that referenced
this pull request
Mar 16, 2026
#82) ft. ENG-23: cross-entity effects, reconciliation, and engine hardening - Replace obligation effect stubs with real cross-entity dispatch (emitObligationOverdue/Settled now read obligation → transition mortgage) - Add transitionMortgageInternal for scheduler/effect use - Add confirmObligationPayment (authedMutation + obligation:manage) - Add notifyAdminNewRequest as entry action on pending_review state - Filter assign-action warnings in effect scheduler (isBuiltInAction) - Extend reconciliation to handle mortgage + obligation entity types - Add machineContext explanatory comments to governed table schemas - Fix reconciliation test to use non-governed entity type (deal) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> ft. ENG-23: seed mutations for all Phase 1 entities - Create convex/seed/ directory with 8 files - seedBroker: 2 brokers with FSRA license numbers, Ontario-based - seedBorrower: 5 borrowers with Ontario addresses, verified IDV - seedLender: 3 lenders with accreditation, broker-assigned - seedMortgage: 5 mortgages with varied terms + auto-generated obligations - generateObligations: interest-only bridge loan payment schedule - seedObligationStates: transitions obligations to due/overdue/settled mix - seedOnboardingRequest: 3 requests (pending_review, approved, rejected) - seedAll: orchestrator in dependency order, idempotent All seeds use executeTransition for non-initial states (proper journal entries). Each entity gets a SEED_CREATED journal entry for audit trail continuity. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> ft. ENG-23: reconciliation cron + internal query - Add daily reconciliation cron (06:00 UTC) that runs Layer 1 check - Create reconciliationAction.ts with internalQuery (no auth required) and internalAction for cron scheduling - Discrepancies logged as P0 errors via console.error - On-demand adminQuery reconciliation remains available Resolves SPEC Open Question #5: daily cron + on-demand admin query. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> ft. ENG-23: integration tests — guard failure, missing effects, cross-entity - Add guard failure test: mortgage DEFAULT_THRESHOLD_REACHED with 0 missed payments → rejected, status unchanged - Add missing effect handler test: unknown action → warn + succeed - Create crossEntity.test.ts with 4 tests proving full obligation→mortgage cascade: overdue dispatch, settlement cure, and journal chain consistency - Fix pre-existing test: "throws when no machine is registered" assertion Cross-entity tests manually invoke effects (convex-test doesn't auto-run scheduled functions), verifying the complete chain: obligation due→overdue → mortgage active→delinquent obligation overdue→settled → mortgage delinquent→active (cure) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> docs: ENG-23 — update SPEC 1.2 and PRD 1.2 on Notion SPEC 1.2 updates: - §2 File Structure: corrected paths to convex/engine/ nesting - §9 Developer Checklist: fixed all file paths - §10 Open Questions: closed #1 (guard timing), #2 (confirmObligationPayment), #3 (generateObligations helper) - Removed effects/mortgage.ts, added effects/workosProvisioning.ts PRD 1.2 updates: - GT DoD #3: marked deal machine as descoped to Project 4 - Renamed seedInvestor → seedLender (schema table is lenders) - Marked seedDeal as descoped Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> fix: ENG-23 — type reconciliationAction entityType parameter Use keyof typeof ENTITY_TABLE_MAP instead of string for type safety in collectLatestEntries function parameter. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> fix: ENG-23 — address code review findings in seed mutations - Fix idempotency: reuse existing user when entity doesn't exist yet (seedBorrower, seedLender, seedOnboardingRequest, seedMortgage) - Fix seedMortgage: reuse existing property instead of duplicating - Fix generateObligations: clamp day to prevent setMonth date drift - Note: seedLender entityType "borrower" is intentional — lenders are not yet a governed entity type in the GT engine Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> ## Summary by Sourcery Implement cross-entity coordination between obligations and mortgages, extend reconciliation and scheduling infrastructure, and add idempotent seed data plus tests and cron-based health checks. New Features: - Enable cross-entity effects so obligation state changes drive corresponding mortgage transitions. - Introduce internal and admin-facing commands for mortgage transitions and confirming obligation payments. - Add an internal reconciliation query and daily cron-driven reconciliation check across governed entities. - Seed phase-1 entities (brokers, borrowers, lenders, mortgages, obligations, onboarding requests) with realistic, journal-backed demo data. - Provide an orchestrated seedAll mutation to populate a complete demo dataset in dependency order. Bug Fixes: - Ensure reconciliation skips non-governed entity types while handling mortgages and obligations correctly. - Prevent scheduling of XState built-in actions as effects and gracefully handle missing effect handlers. - Fix reconciliation tests to use non-governed entities and relax an over-specific transition engine error assertion. - Clamp generated obligation due dates to avoid calendar drift in monthly schedules. - Improve seed mutations to be idempotent by reusing existing users, properties, and entities. Enhancements: - Document machineContext usage for governed tables and add notification entry actions to onboarding requests. - Expand reconciliation utilities to support internal use and improve discrepancy reporting. - Strengthen guard behavior for mortgage default thresholds via explicit tests. Documentation: - Clarify governed vs non-governed entities and machine context expectations in schema comments. Tests: - Add cross-entity integration tests covering obligation→mortgage cascades and journal chain consistency. - Add tests for guard failures, missing effect handlers, and reconciliation behavior across entity types. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **New Features** * Added automated daily health checks to verify system integrity. * Enabled obligation payment confirmation through the admin dashboard. * Added admin notifications for new onboarding requests. * **Improvements** * Enhanced coordination between related financial entities to ensure consistent state transitions. * Strengthened validation to prevent invalid state changes. * **Tests** * Added comprehensive test coverage for entity workflows and state validation. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Connorbelez
added a commit
that referenced
this pull request
Mar 28, 2026
- Wrap handlePipelineLegConfirmed in try/catch to protect cash postings (#1, #11) - Make initiateTransferInternal retry-safe with early return (#2, #5, #10) - Fix misleading comment in startDealClosingPipeline (#4) - Add validatePipelineFields guard to public createTransferRequest (#7, #12) - Document supported provider codes for deal-closing (#8) - Filter pipeline legs by specific pipelineId in getPipelineStatus (#6) - Return null for unknown pipeline IDs in getTransfersByPipeline (#15) - Validate leg2Amount early in createDealClosingPipeline (#13) - computePipelineStatus prefers active legs over terminal after retry (#14) - Updated tests for retry scenarios (37 passing) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Connorbelez
added a commit
that referenced
this pull request
Apr 20, 2026
- Hash-chain rejected transitions for complete audit evidence - Derive event types from machine config instead of hardcoding - Validate payload is plain object before spreading into XState event - Add take(200) limits and index-ordered queries to prevent unbounded reads - Add by_created index on demo_gt_entities for ordered listing - Trim whitespace in hasCompleteData guard + add POC TODO comment - Validate loan amount is finite & positive before mutation - Wrap Seed/Lifecycle buttons in try/catch error handling - Derive journal stats from filtered data so counts match visible rows - Add aria-label/expanded/controls to filter toggle for accessibility - Guard editing when external props are supplied (half-controlled fix) - Handle empty node arrays in Math.max content size init - Use more specific regex assertion in e2e test - Align design.md guard description and IP field with implementation - Sync chunk task completion status and fix e2e test path in specs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Connorbelez
added a commit
that referenced
this pull request
Apr 20, 2026
## Summary - **XState-based state machine engine** for loan applications with governed transitions (submit → review → approve/reject → fund → close) - **Audit infrastructure**: hash-chained journal via `convex-audit-log` + `AuditTrail` component, structured `auditLog.log()` calls across all mutations - **Backend**: fluent-convex queries/mutations with indexed reads, bounded `.take()` pagination, composite indexes, effect registry with dedicated `internalMutation` handlers - **Frontend**: TanStack Router pages — Command Center (entity CRUD + transition controls), Journal Viewer (filtered journal with stats), Machine Inspector (state diagram + transition table) - **E2E tests**: Playwright suite covering navigation, entity lifecycle, rejected transitions, full lifecycle, journal verification, and machine inspector ## Files changed - `convex/schema.ts` — 3 new `demo_gt_*` tables with indexes - `convex/demo/governedTransitions.ts` — engine, mutations, queries - `convex/demo/governedTransitionsEffects.ts` — effect registry handlers - `convex/demo/machines/loanApplication.machine.ts` — XState v5 machine definition - `convex/demo/machines/registry.ts` — machine registry - `src/routes/demo/governed-transitions/` — 5 route files + 2 view components - `src/components/ui/` — interactive-logs-table and n8n-workflow-block improvements - `e2e/governed-transitions.spec.ts` — comprehensive E2E test suite - `specs/governed-transitions/` — design doc, task tracking, chunked context ## Test plan - [x] `bun check` passes (biome lint + format) - [x] `bun typecheck` passes (TypeScript) - [x] `bunx convex codegen` passes (generated types) - [ ] `bun test` — unit tests for loanApplication machine - [ ] `bun test:e2e` — full E2E suite 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **New Features** * Added Governed Transitions demo showcasing a complete loan application workflow with state machine-driven transitions * Introduced Command Center for creating applications and triggering transitions * Added Journal Viewer to inspect all state changes and outcomes * Added Machine Inspector to visualize workflow topology and valid transitions * New demo accessible via Platform menu in header * **Tests** * Added comprehensive end-to-end test coverage for core workflows * Added unit tests for state machine transitions <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Connorbelez
added a commit
that referenced
this pull request
Apr 20, 2026
- Add daily reconciliation cron (06:00 UTC) that runs Layer 1 check - Create reconciliationAction.ts with internalQuery (no auth required) and internalAction for cron scheduling - Discrepancies logged as P0 errors via console.error - On-demand adminQuery reconciliation remains available Resolves SPEC Open Question #5: daily cron + on-demand admin query. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Connorbelez
added a commit
that referenced
this pull request
Apr 20, 2026
#82) ft. ENG-23: cross-entity effects, reconciliation, and engine hardening - Replace obligation effect stubs with real cross-entity dispatch (emitObligationOverdue/Settled now read obligation → transition mortgage) - Add transitionMortgageInternal for scheduler/effect use - Add confirmObligationPayment (authedMutation + obligation:manage) - Add notifyAdminNewRequest as entry action on pending_review state - Filter assign-action warnings in effect scheduler (isBuiltInAction) - Extend reconciliation to handle mortgage + obligation entity types - Add machineContext explanatory comments to governed table schemas - Fix reconciliation test to use non-governed entity type (deal) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> ft. ENG-23: seed mutations for all Phase 1 entities - Create convex/seed/ directory with 8 files - seedBroker: 2 brokers with FSRA license numbers, Ontario-based - seedBorrower: 5 borrowers with Ontario addresses, verified IDV - seedLender: 3 lenders with accreditation, broker-assigned - seedMortgage: 5 mortgages with varied terms + auto-generated obligations - generateObligations: interest-only bridge loan payment schedule - seedObligationStates: transitions obligations to due/overdue/settled mix - seedOnboardingRequest: 3 requests (pending_review, approved, rejected) - seedAll: orchestrator in dependency order, idempotent All seeds use executeTransition for non-initial states (proper journal entries). Each entity gets a SEED_CREATED journal entry for audit trail continuity. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> ft. ENG-23: reconciliation cron + internal query - Add daily reconciliation cron (06:00 UTC) that runs Layer 1 check - Create reconciliationAction.ts with internalQuery (no auth required) and internalAction for cron scheduling - Discrepancies logged as P0 errors via console.error - On-demand adminQuery reconciliation remains available Resolves SPEC Open Question #5: daily cron + on-demand admin query. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> ft. ENG-23: integration tests — guard failure, missing effects, cross-entity - Add guard failure test: mortgage DEFAULT_THRESHOLD_REACHED with 0 missed payments → rejected, status unchanged - Add missing effect handler test: unknown action → warn + succeed - Create crossEntity.test.ts with 4 tests proving full obligation→mortgage cascade: overdue dispatch, settlement cure, and journal chain consistency - Fix pre-existing test: "throws when no machine is registered" assertion Cross-entity tests manually invoke effects (convex-test doesn't auto-run scheduled functions), verifying the complete chain: obligation due→overdue → mortgage active→delinquent obligation overdue→settled → mortgage delinquent→active (cure) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> docs: ENG-23 — update SPEC 1.2 and PRD 1.2 on Notion SPEC 1.2 updates: - §2 File Structure: corrected paths to convex/engine/ nesting - §9 Developer Checklist: fixed all file paths - §10 Open Questions: closed #1 (guard timing), #2 (confirmObligationPayment), #3 (generateObligations helper) - Removed effects/mortgage.ts, added effects/workosProvisioning.ts PRD 1.2 updates: - GT DoD #3: marked deal machine as descoped to Project 4 - Renamed seedInvestor → seedLender (schema table is lenders) - Marked seedDeal as descoped Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> fix: ENG-23 — type reconciliationAction entityType parameter Use keyof typeof ENTITY_TABLE_MAP instead of string for type safety in collectLatestEntries function parameter. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> fix: ENG-23 — address code review findings in seed mutations - Fix idempotency: reuse existing user when entity doesn't exist yet (seedBorrower, seedLender, seedOnboardingRequest, seedMortgage) - Fix seedMortgage: reuse existing property instead of duplicating - Fix generateObligations: clamp day to prevent setMonth date drift - Note: seedLender entityType "borrower" is intentional — lenders are not yet a governed entity type in the GT engine Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> ## Summary by Sourcery Implement cross-entity coordination between obligations and mortgages, extend reconciliation and scheduling infrastructure, and add idempotent seed data plus tests and cron-based health checks. New Features: - Enable cross-entity effects so obligation state changes drive corresponding mortgage transitions. - Introduce internal and admin-facing commands for mortgage transitions and confirming obligation payments. - Add an internal reconciliation query and daily cron-driven reconciliation check across governed entities. - Seed phase-1 entities (brokers, borrowers, lenders, mortgages, obligations, onboarding requests) with realistic, journal-backed demo data. - Provide an orchestrated seedAll mutation to populate a complete demo dataset in dependency order. Bug Fixes: - Ensure reconciliation skips non-governed entity types while handling mortgages and obligations correctly. - Prevent scheduling of XState built-in actions as effects and gracefully handle missing effect handlers. - Fix reconciliation tests to use non-governed entities and relax an over-specific transition engine error assertion. - Clamp generated obligation due dates to avoid calendar drift in monthly schedules. - Improve seed mutations to be idempotent by reusing existing users, properties, and entities. Enhancements: - Document machineContext usage for governed tables and add notification entry actions to onboarding requests. - Expand reconciliation utilities to support internal use and improve discrepancy reporting. - Strengthen guard behavior for mortgage default thresholds via explicit tests. Documentation: - Clarify governed vs non-governed entities and machine context expectations in schema comments. Tests: - Add cross-entity integration tests covering obligation→mortgage cascades and journal chain consistency. - Add tests for guard failures, missing effect handlers, and reconciliation behavior across entity types. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes * **New Features** * Added automated daily health checks to verify system integrity. * Enabled obligation payment confirmation through the admin dashboard. * Added admin notifications for new onboarding requests. * **Improvements** * Enhanced coordination between related financial entities to ensure consistent state transitions. * Strengthened validation to prevent invalid state changes. * **Tests** * Added comprehensive test coverage for entity workflows and state validation. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
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
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.
Summary
convex-audit-log+AuditTrailcomponent, structuredauditLog.log()calls across all mutations.take()pagination, composite indexes, effect registry with dedicatedinternalMutationhandlersFiles changed
convex/schema.ts— 3 newdemo_gt_*tables with indexesconvex/demo/governedTransitions.ts— engine, mutations, queriesconvex/demo/governedTransitionsEffects.ts— effect registry handlersconvex/demo/machines/loanApplication.machine.ts— XState v5 machine definitionconvex/demo/machines/registry.ts— machine registrysrc/routes/demo/governed-transitions/— 5 route files + 2 view componentssrc/components/ui/— interactive-logs-table and n8n-workflow-block improvementse2e/governed-transitions.spec.ts— comprehensive E2E test suitespecs/governed-transitions/— design doc, task tracking, chunked contextTest plan
bun checkpasses (biome lint + format)bun typecheckpasses (TypeScript)bunx convex codegenpasses (generated types)bun test— unit tests for loanApplication machinebun test:e2e— full E2E suite🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Tests