Skip to content

feat(home): seed onboarding-sourced facts into relationship-state [JARVIS-471]#25434

Merged
alex-nork merged 1 commit into
mainfrom
alex-nork/jarvis-471-onboarding-facts
Apr 14, 2026
Merged

feat(home): seed onboarding-sourced facts into relationship-state [JARVIS-471]#25434
alex-nork merged 1 commit into
mainfrom
alex-nork/jarvis-471-onboarding-facts

Conversation

@alex-nork

@alex-nork alex-nork commented Apr 14, 2026

Copy link
Copy Markdown
Contributor

Summary

Phase 4 backend of the Onboarding & New User Retention TDD. On the very first message of a fresh conversation, persist the pre-chat onboarding payload so the Home page renders dashed-border onboarding chips immediately.

What changed

  • assistant/src/home/relationship-state-writer.ts — new writeOnboardingSidecar() / getOnboardingSidecarPath() exports + a private readOnboardingSidecar(). extractFacts() now accepts an onboarding? arg and emits tools → world / tasks → priorities / tone → voice facts tagged source: "onboarding" (rendered first so they lead the Home chip list). computeRelationshipState() reads the sidecar and falls back to its userName / assistantName when IDENTITY.md / USER.md haven't yielded names yet.
  • assistant/src/runtime/routes/conversation-routes.ts — new persistOnboardingArtifacts() helper called from the existing body.onboarding && conversation.messages.length === 0 gate in handleSendMessage. Writes the sidecar, seeds IDENTITY.md / USER.md only when missing (never clobbers existing persona content), and kicks writeRelationshipState() fire-and-forget so the Home page populates on first visit.
  • assistant/src/home/__tests__/relationship-state-writer.test.ts — new onboarding sidecar (JARVIS-471) block with 9 tests.

Critical design note

The relationship-state writer is a pure recomputation on every turn boundary — it reads USER.md / SOUL.md / IDENTITY.md from disk and rebuilds facts[] from scratch, never merging with the on-disk JSON. Without a durable source for onboarding selections, chips written on the first message would vanish on the second turn. The sidecar (data/onboarding-context.json) is exactly that durable source — read on every recomputation so onboarding facts survive restarts and subsequent writes.

IDENTITY.md / USER.md are seeded from onboarding too, but only when missing, so we never clobber existing persona content. They're the durable source for the system prompt and the writer's parseIdentity / parseUserName helpers after a daemon restart drops the in-memory conversation.onboardingContext.

No frontend changes

Phase 3 already baked in the dashed-border source: "onboarding" fact chips + the empty-state "Start chatting and I'll pick up a lot more" nudge as forward-compat for Phase 4. The macOS client renders this PR's new facts correctly with no additional work. LUM-806 (Jason's frontend ticket) is effectively superseded by Phase 3's forward-compat.

Test plan

  • bun test src/home/__tests__/relationship-state-writer.test.ts — 40 tests pass (9 new + 31 existing)
  • bun run typecheck — clean
  • Pre-push type-check + lint + Swift build — passes
  • Hand-test: Pax launches cleanly on macOS Tahoe (after fix(macos): guard SSE task against superseded session on MainActor race #25426 merged the SSE guard fix that unblocked testing)
  • Hand-test: toggle home-tab on, run pre-chat onboarding, confirm Home tab shows 4–8 dashed-border chips immediately on first visit
  • Hand-test: second turn of the same conversation does not vanish the onboarding chips
  • Hand-test: daemon + app restart after first turn preserves both onboarding chips (sidecar durability) and assistantName/userName (IDENTITY.md / USER.md seed)

🤖 Generated with Claude Code

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 4 additional findings.

Open in Devin Review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c5e69922e2

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

}): void {
writeOnboardingSidecar(onboarding);

const assistantName = onboarding.assistantName?.trim();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Guard onboarding names before calling trim

handleSendMessage treats req.json() as a typed object but never validates onboarding at runtime, so onboarding.assistantName?.trim() and onboarding.userName?.trim() can throw when those fields are non-strings (for example, a buggy or older client sending numbers). In that case the first message request fails before enqueueing, which contradicts the helper’s “never throws” contract; add a typeof === "string" guard (or schema validation) before trimming.

Useful? React with 👍 / 👎.

Comment on lines +139 to +140
if (!parsed || !Array.isArray(parsed.tools) || !Array.isArray(parsed.tasks))
return null;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Validate sidecar field types before using string methods

readOnboardingSidecar() only checks that tools and tasks are arrays, but computeRelationshipState() later assumes every entry (and tone) is a string and calls .trim() on them. A malformed persisted payload (e.g. numeric tool/tone values) will throw during state computation, causing live home-state computation to fail and relationship-state writes to degrade. The sidecar parser should validate/sanitize element types (and tone/name types) before returning data.

Useful? React with 👍 / 👎.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 new potential issue.

View 7 additional findings in Devin Review.

Open in Devin Review

// loop has unwound (whether via success, error, or
// cancellation), so the AsyncBytes iterator on the
// cooperative thread pool is never freed out from under
// itself (preserves PR #25396's EXC_BAD_ACCESS fix).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Comment references past PR number, violating clients/AGENTS.md comment quality rules

The comment at line 280 includes (preserves PR #25396's EXC_BAD_ACCESS fix), which narrates code history by referencing a past PR. clients/AGENTS.md § "Comment Quality" states: "Comments and docstrings must describe the code's intent and behavior, not its refactoring history." The comment should describe the current behavior (why defer is needed) without referencing past PRs — future readers shouldn't need to know about PR #25396 to understand why the code is structured this way.

Suggested change
// itself (preserves PR #25396's EXC_BAD_ACCESS fix).
// itself.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

…RVIS-471]

Phase 4 backend of the Onboarding & New User Retention TDD.

On the very first message of a fresh conversation, persist the
pre-chat onboarding payload so the Home page renders dashed-border
onboarding chips immediately instead of waiting for the first
inferred-fact pass.

Three artifacts get produced from the onboarding JSON:

- `data/onboarding-context.json` — durable sidecar read on every
  `computeRelationshipState()` call. Required because the
  relationship-state writer is a pure recomputation on every turn
  boundary — without the sidecar, onboarding chips would vanish
  on turn 2.
- `IDENTITY.md` / `USER.md` — seed files, only written when missing
  so we never clobber existing persona content. Feed the system
  prompt and the writer's `parseIdentity` / `parseUserName` helpers
  after a daemon restart when the in-memory onboarding context is
  gone.
- `data/relationship-state.json` — kicked off fire-and-forget via
  the existing `writeRelationshipState()` entry so the Home page
  can populate on first visit instead of waiting for the first
  agent-turn boundary.

Onboarding facts render first in the chip list (tools -> world,
tasks -> priorities, tone -> voice), tagged `source: "onboarding"`
so the frontend applies its dashed-border styling. Sidecar
userName / assistantName now also fill in as fallbacks when
IDENTITY.md / USER.md have not yielded names yet.

Nine new tests cover sidecar path, write, fact extraction, name
fallbacks + precedence, onboarding/inferred coexistence, empty
input skipping, missing-sidecar behavior, and corrupt-JSON
degradation.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@alex-nork alex-nork force-pushed the alex-nork/jarvis-471-onboarding-facts branch from 0e6ae0a to 49cc970 Compare April 14, 2026 12:24
@alex-nork alex-nork merged commit 550500d into main Apr 14, 2026
12 checks passed
@alex-nork alex-nork deleted the alex-nork/jarvis-471-onboarding-facts branch April 14, 2026 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant