feat(persona): guided persona builder over SOUL.md (#4253, PR1)#4412
feat(persona): guided persona builder over SOUL.md (#4253, PR1)#4412M3gA-Mind wants to merge 2 commits into
Conversation
Add a structured, non-technical persona editor that maps friendly fields (Personality, Communication style, About you) to named SOUL.md sections and splices them in place, keeping SOUL.md the runtime source of truth. Guided is the default; the raw markdown editor stays behind an Advanced toggle. Reuses the existing workspace_file_read/write/reset RPC — no core changes. Part of the phased tinyhumansai#4253 work (PR1 of N).
📝 WalkthroughWalkthroughAdds a guided/advanced editing mode for SOUL.md persona files. Introduces a lossless managed-section parser/writer utility, a new ChangesGuided persona editor
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant PersonaPanel
participant PersonaGuidedFields
participant personaSections
User->>PersonaPanel: toggle to guided mode
PersonaPanel->>PersonaGuidedFields: render(soulDraft)
PersonaGuidedFields->>personaSections: parsePersonaFields(soulDraft)
personaSections-->>PersonaGuidedFields: PersonaFields
User->>PersonaGuidedFields: edit personality/voice/about field
PersonaGuidedFields->>personaSections: applyPersonaField(soul, key, value)
personaSections-->>PersonaGuidedFields: updated soul text
PersonaGuidedFields-->>PersonaPanel: onChange(updatedSoul)
User->>PersonaPanel: click Save
PersonaPanel->>PersonaPanel: writePersonaFile RPC
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 941d38a2fd
ℹ️ 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".
| */ | ||
| export function applyPersonaField(soul: string, key: PersonaFieldKey, value: string): string { | ||
| const heading = HEADING_FOR[key]; | ||
| const nextBody = value.trim(); |
There was a problem hiding this comment.
In guided mode this helper runs on every textarea onChange, so trimming the draft before splicing immediately discards any trailing newline the user just typed. When a user presses Enter at the end of the Personality or Communication style field to add another paragraph/bullet, readSection(...) === nextBody can return the original SOUL.md and React re-renders without the newline, causing the next characters to be appended to the previous line. Since the default managed sections are multiline lists, preserve the live value here and only normalize for comparison/save if needed.
Useful? React with 👍 / 👎.
Prettier-only formatting of the persona builder components/tests and the guided-persona i18n key additions across all locale files.
|
Fixed Frontend Checks. The lane failed at Verified locally, all green: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
app/src/components/settings/panels/persona/personaSections.ts (1)
35-39: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
HEADING_FORduplicates data already inPERSONA_SECTIONS.Both structures map the same keys to the same heading strings. Consider deriving
HEADING_FORfromPERSONA_SECTIONSto avoid the two falling out of sync if a heading is ever renamed.♻️ Proposed refactor
-const HEADING_FOR: Record<PersonaFieldKey, string> = { - personality: 'Personality', - voice: 'Voice', - about: 'About You', -}; +const HEADING_FOR: Record<PersonaFieldKey, string> = Object.fromEntries( + PERSONA_SECTIONS.map(({ key, heading }) => [key, heading]) +) as Record<PersonaFieldKey, string>;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/components/settings/panels/persona/personaSections.ts` around lines 35 - 39, HEADING_FOR currently repeats the same heading strings already defined in PERSONA_SECTIONS, so keep the mappings in sync by deriving HEADING_FOR from PERSONA_SECTIONS instead of hardcoding a second source of truth. Update personaSections.ts so the heading lookup is built from PERSONA_SECTIONS using the existing PersonaFieldKey entries, and preserve the current values for personality, voice, and about while reusing the same symbols.app/src/components/settings/panels/persona/personaSections.test.ts (1)
67-72: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding a regression test for the clear→refill whitespace-growth case.
The current "empties the body but keeps the heading" test doesn't cover refilling after clearing, which is where the lead/trail overlap bug in
applyPersonaField(seepersonaSections.ts) manifests. A test likeapplyPersonaField(applyPersonaField(SOUL, 'voice', ''), 'voice', 'Be terse.')should assert the surrounding whitespace matches a direct apply.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/components/settings/panels/persona/personaSections.test.ts` around lines 67 - 72, Add a regression test in personaSections.test.ts for the clear-then-refill case around applyPersonaField: after clearing the 'voice' field from SOUL, apply a new non-empty value like 'Be terse.' and assert the resulting text matches the same output as a direct apply to SOUL. Use parsePersonaFields and applyPersonaField in the existing personaSections test suite to verify the surrounding whitespace/section spacing stays identical and does not grow after the clear→refill path.app/src/components/settings/panels/PersonaPanel.tsx (1)
208-225: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider a small data-driven render for the two mode buttons.
The guided/advanced buttons are nearly identical (only variant/aria-pressed/onClick/label differ). Could be collapsed into a
.map()over a small config array for less duplication, but this is purely cosmetic given there are only two buttons.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/src/components/settings/panels/PersonaPanel.tsx` around lines 208 - 225, The guided and advanced mode buttons in PersonaPanel are duplicated aside from a few props, so replace the repeated Button blocks with a small data-driven render using a config array and .map(). Keep the existing behavior intact by preserving the current soulMode checks, setSoulMode calls, test IDs, and translated labels while moving the shared Button markup into one reusable path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/components/settings/panels/persona/personaSections.ts`:
- Around line 102-106: The live update path in applyPersonaField is trimming the
incoming textarea value on every keystroke, which breaks typing of trailing
spaces/newlines in PersonaGuidedFields. Update applyPersonaField so
readSection/HEADING_FOR is still used to compare normalized content for the
no-op check, but preserve the raw value when writing the updated section back
into the document; defer trimming to save/serialize time instead of applying it
to every onChange update.
- Around line 108-120: The whitespace-preservation logic in personaSections.ts
is double-counting newlines when a section body is only whitespace, because the
lead and trail matches overlap on the same raw string. Update the section
replacement path in the function that uses findSectionSpan and splices nextBody
so that leading/trailing newline capture is computed non-overlapping or
normalized when raw contains only newlines, ensuring repeated clear/refill
cycles stay idempotent and do not grow whitespace.
---
Nitpick comments:
In `@app/src/components/settings/panels/persona/personaSections.test.ts`:
- Around line 67-72: Add a regression test in personaSections.test.ts for the
clear-then-refill case around applyPersonaField: after clearing the 'voice'
field from SOUL, apply a new non-empty value like 'Be terse.' and assert the
resulting text matches the same output as a direct apply to SOUL. Use
parsePersonaFields and applyPersonaField in the existing personaSections test
suite to verify the surrounding whitespace/section spacing stays identical and
does not grow after the clear→refill path.
In `@app/src/components/settings/panels/persona/personaSections.ts`:
- Around line 35-39: HEADING_FOR currently repeats the same heading strings
already defined in PERSONA_SECTIONS, so keep the mappings in sync by deriving
HEADING_FOR from PERSONA_SECTIONS instead of hardcoding a second source of
truth. Update personaSections.ts so the heading lookup is built from
PERSONA_SECTIONS using the existing PersonaFieldKey entries, and preserve the
current values for personality, voice, and about while reusing the same symbols.
In `@app/src/components/settings/panels/PersonaPanel.tsx`:
- Around line 208-225: The guided and advanced mode buttons in PersonaPanel are
duplicated aside from a few props, so replace the repeated Button blocks with a
small data-driven render using a config array and .map(). Keep the existing
behavior intact by preserving the current soulMode checks, setSoulMode calls,
test IDs, and translated labels while moving the shared Button markup into one
reusable path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f3e09482-8e59-4ab1-b56e-d4890f5793d1
📒 Files selected for processing (19)
app/src/components/settings/panels/PersonaPanel.test.tsxapp/src/components/settings/panels/PersonaPanel.tsxapp/src/components/settings/panels/persona/PersonaGuidedFields.tsxapp/src/components/settings/panels/persona/personaSections.test.tsapp/src/components/settings/panels/persona/personaSections.tsapp/src/lib/i18n/ar.tsapp/src/lib/i18n/bn.tsapp/src/lib/i18n/de.tsapp/src/lib/i18n/en.tsapp/src/lib/i18n/es.tsapp/src/lib/i18n/fr.tsapp/src/lib/i18n/hi.tsapp/src/lib/i18n/id.tsapp/src/lib/i18n/it.tsapp/src/lib/i18n/ko.tsapp/src/lib/i18n/pl.tsapp/src/lib/i18n/pt.tsapp/src/lib/i18n/ru.tsapp/src/lib/i18n/zh-CN.ts
| export function applyPersonaField(soul: string, key: PersonaFieldKey, value: string): string { | ||
| const heading = HEADING_FOR[key]; | ||
| const nextBody = value.trim(); | ||
|
|
||
| if (readSection(soul, heading) === nextBody) return soul; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Trimming value on every call breaks live typing when this is wired to onChange per keystroke.
nextBody = value.trim() strips trailing whitespace/newlines unconditionally. PersonaGuidedFields calls onChange(applyPersonaField(value, field.key, e.target.value)) on every keystroke, and the textarea's displayed value comes back from parsePersonaFields(value) (which also trims via readSection). As a result, any trailing space or blank line the user types is immediately stripped on the very next render — the user can't type a trailing space/blank line, and the controlled textarea's committed value diverging from what was just typed can cause visible flicker or cursor-position glitches mid-typing.
Trimming is appropriate for save/serialize, but shouldn't run on every interim keystroke update. Consider trimming only when the value actually differs after normalization for comparison purposes, but preserving the untrimmed live value in the spliced document (only trim for the equality short-circuit check, not for what gets written back), or defer trimming to blur/save time in the consuming component.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/components/settings/panels/persona/personaSections.ts` around lines
102 - 106, The live update path in applyPersonaField is trimming the incoming
textarea value on every keystroke, which breaks typing of trailing
spaces/newlines in PersonaGuidedFields. Update applyPersonaField so
readSection/HEADING_FOR is still used to compare normalized content for the
no-op check, but preserve the raw value when writing the updated section back
into the document; defer trimming to save/serialize time instead of applying it
to every onChange update.
| const span = findSectionSpan(soul, heading); | ||
| if (span) { | ||
| const raw = soul.slice(span.bodyStart, span.bodyEnd); | ||
| const lead = raw.match(/^\n*/)?.[0] ?? ''; | ||
| const trail = raw.match(/\n*$/)?.[0] ?? ''; | ||
| const spliced = nextBody ? `${lead}${nextBody}${trail || '\n'}` : `${lead}${trail}`; | ||
| return soul.slice(0, span.bodyStart) + spliced + soul.slice(span.bodyEnd); | ||
| } | ||
|
|
||
| if (!nextBody) return soul; | ||
| const base = soul.replace(/\n*$/, '\n'); | ||
| return `${base}\n## ${heading}\n\n${nextBody}\n`; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Lead/trail newline capture overlaps when the section body is pure whitespace, causing whitespace to double on each clear→refill cycle.
When raw consists entirely of newlines (e.g., right after clearing a section with applyPersonaField(soul, key, '')), both raw.match(/^\n*/) and raw.match(/\n*$/) independently match the entire string (not disjoint halves), so lead and trail each duplicate the same newlines. Concrete trace using the section fixture in the test file:
- Clear
voice→ body becomes"\n\n\n"(lead"\n"+ trail"\n\n", concatenated, 3 chars total — correct so far). - Refill
voicewith'Be terse.'→rawis now the pure-whitespace"\n\n\n";leadandtraileach match all 3 chars independently, so the new body becomes"\n\n\n" + "Be terse." + "\n\n\n"(6 newlines) instead of the expected 3 that a direct apply would produce.
Each subsequent clear/refill cycle re-doubles the surrounding whitespace, silently growing the document. This breaks the "lossless and idempotent" guarantee described in the file's own docstring.
🐛 Proposed fix: prevent lead/trail overlap when raw is pure whitespace
const span = findSectionSpan(soul, heading);
if (span) {
const raw = soul.slice(span.bodyStart, span.bodyEnd);
const lead = raw.match(/^\n*/)?.[0] ?? '';
- const trail = raw.match(/\n*$/)?.[0] ?? '';
+ const trail = lead.length >= raw.length ? '' : raw.match(/\n*$/)?.[0] ?? '';
const spliced = nextBody ? `${lead}${nextBody}${trail || '\n'}` : `${lead}${trail}`;
return soul.slice(0, span.bodyStart) + spliced + soul.slice(span.bodyEnd);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const span = findSectionSpan(soul, heading); | |
| if (span) { | |
| const raw = soul.slice(span.bodyStart, span.bodyEnd); | |
| const lead = raw.match(/^\n*/)?.[0] ?? ''; | |
| const trail = raw.match(/\n*$/)?.[0] ?? ''; | |
| const spliced = nextBody ? `${lead}${nextBody}${trail || '\n'}` : `${lead}${trail}`; | |
| return soul.slice(0, span.bodyStart) + spliced + soul.slice(span.bodyEnd); | |
| } | |
| if (!nextBody) return soul; | |
| const base = soul.replace(/\n*$/, '\n'); | |
| return `${base}\n## ${heading}\n\n${nextBody}\n`; | |
| } | |
| const span = findSectionSpan(soul, heading); | |
| if (span) { | |
| const raw = soul.slice(span.bodyStart, span.bodyEnd); | |
| const lead = raw.match(/^\n*/)?.[0] ?? ''; | |
| const trail = lead.length >= raw.length ? '' : raw.match(/\n*$/)?.[0] ?? ''; | |
| const spliced = nextBody ? `${lead}${nextBody}${trail || '\n'}` : `${lead}${trail}`; | |
| return soul.slice(0, span.bodyStart) + spliced + soul.slice(span.bodyEnd); | |
| } | |
| if (!nextBody) return soul; | |
| const base = soul.replace(/\n*$/, '\n'); | |
| return `${base}\n## ${heading}\n\n${nextBody}\n`; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/src/components/settings/panels/persona/personaSections.ts` around lines
108 - 120, The whitespace-preservation logic in personaSections.ts is
double-counting newlines when a section body is only whitespace, because the
lead and trail matches overlap on the same raw string. Update the section
replacement path in the function that uses findSectionSpan and splices nextBody
so that leading/trailing newline capture is computed non-overlapping or
normalized when raw contains only newlines, ensuring repeated clear/refill
cycles stay idempotent and do not grow whitespace.
|
Also ticked the PR Submission Checklist honestly (it was the last soft-gate red). The 3 open items were all verified: |
1 participant
Summary
SOUL.md.##sections and are spliced in place, soSOUL.mdstays the single source of truth the runtime injects. Every other byte (title, intro, hand-written sections) is preserved.openhuman.workspace_file_read/write/resetRPC — no Rust/core changes.Problem
Feedback on #4253: the
SOUL.mdconcept is strong, but users don't know how to write a good persona. The editor today is a raw markdown textarea. Non-technical users need a structured way to shape identity/behavior without understanding the file format — while keeping the assistant runtime's source of truth intact (not disconnected UI-only state).Solution
personaSections.ts: a lossless, idempotent parser/serializer.parsePersonaFieldsreads the managed sections;applyPersonaFieldreplaces only the target section's body (preserving surrounding whitespace), appends a## About Youblock on demand, and returns the input unchanged when nothing changed. Deeper###headings stay part of a section; matching is exact and case-insensitive.PersonaGuidedFields.tsx: the structured form. Persona = identity/behavior prose only; it links out to Settings → Agent access for permissions/tools rather than duplicating that config.PersonaPanel.tsx: Guided/Advanced mode toggle sharing one Save/Reset path; the raw text remains the single working value both modes edit.settings.persona.builder.*keys added toenand all 13 locale files (parity 13/13 each).Submission Checklist
personaSections.test.ts(parser: parse, idempotency, splice-preserves-other-bytes, append, clear, heading edge cases, round-trip) and extendedPersonaPanel.test.tsx(guided default, guided-splice-save over RPC, Advanced raw-edit save, Agent-access link, reset/error paths).personaSections.ts) and UI (PersonaGuidedFields/PersonaPanel) are exercised by the tests above. Please confirm in CI.N/A: no new feature ID; enhances the existing Persona panel.N/A: no matrix feature-behaviour change.N/A: additive Settings UI, no release-cut surface.Closes #NNN— this is PR1 of several; intentionally references (not closes) Guided persona builder and memory dashboard for non-technical users #4253.Impact
Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
Validation Run
pnpm --filter openhuman-app format:check— Prettier--checkpasses; also green in CI (Frontend Checkslane).pnpm typecheck—tsc --noEmitpasses (exit 0); also green in CI (Frontend Checks).personaSections.test.ts+PersonaPanel.test.tsxrun green (22 passed);pnpm i18n:checkalso passes (0 missing/extra across all locales). Confirmed by the passingFrontend ChecksCI lane.Validation Blocked
command:N/Aerror:N/Aimpact:N/ABehavior Changes
Parity Contract
workspace_file_*RPC and allowlist; guided edits produce plainSOUL.mdmarkdown.Duplicate / Superseded PR Handling
Summary by CodeRabbit
New Features
Bug Fixes