Skip to content

Fix default conversation character seed#1165

Merged
cha1latte merged 2 commits into
refactorfrom
fix/refactor-pr-30-default-character-seed
May 25, 2026
Merged

Fix default conversation character seed#1165
cha1latte merged 2 commits into
refactorfrom
fix/refactor-pr-30-default-character-seed

Conversation

@munimunigamer
Copy link
Copy Markdown
Collaborator

@munimunigamer munimunigamer commented May 24, 2026

Migrated from Pasta-Devs/Marinara-Engine-Refactor#30: Pasta-Devs/Marinara-Engine-Refactor#30
Original author: @cha1latte
Target base: Pasta-Devs/Marinara-Engine refactor
Source draft state: False
Verification after port: cargo check --manifest-path src-tauri/Cargo.toml
Port note: source updates/ tracker/evidence files were omitted because the target refactor branch removed repo-local updates guidance.


Summary

Fixes the Discord-sourced setup blocker where fresh Conversation setup had no selectable Professor Mari/default character, leaving the new-chat flow unable to reach Characters: 1.

The default seeding path now creates the built-in __professor_mari__ character when missing, and the legacy cleanup no longer deletes that active default character id on startup.

Proof

Before fix, the controlled setup repro had no selectable Professor Mari result and kept Start Chatting disabled:

Before: Professor Mari unavailable

After fix, Professor Mari appears, can be selected, and the setup flow creates a chat with exactly one character:

After: Professor Mari selected

Validation

  • CARGO_INCREMENTAL=0 cargo test --manifest-path src-tauri/Cargo.toml seeds_professor_mari_as_default_character
  • CARGO_INCREMENTAL=0 pnpm check
  • Fresh Conversation setup can select Professor Mari/default character
  • Brand-new chat reaches Characters: 1
  • Existing chat setup/settings behavior remains in scope for manual smoke verification in real Tauri

Scope Notes

No remote runtime, sync server, browser hosting, auth, Docker, SSE, /api/invoke, AI background remover, drag-and-drop, schema, version, or prompt-pipeline changes.

Summary by CodeRabbit

  • New Features

    • Professor Mari is now bundled as a built-in assistant and automatically initialized when launching the app.
  • Bug Fixes

    • Enhanced cleanup of legacy character data to remove outdated entries more effectively.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 24, 2026

Warning

Review limit reached

@cha1latte, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 19 minutes and 35 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 07660a30-124e-47bf-a06d-a6c232df7e1e

📥 Commits

Reviewing files that changed from the base of the PR and between 2ef16e8 and 0be2c79.

📒 Files selected for processing (1)
  • src-tauri/src/seed_defaults.rs
📝 Walkthrough

Walkthrough

The initialization routine now seeds a bundled "Professor Mari" character by introducing a constant identity, a dedicated seeding function, and integration into the default bundled-data flow. Legacy cleanup is refactored to address only the prior character entry.

Changes

Professor Mari Bundled Character Initialization

Layer / File(s) Summary
Character seeding and legacy cleanup
src-tauri/src/seed_defaults.rs
PROFESSOR_MARI_ID constant defines the canonical identity. seed_professor_mari_character creates the character record with built-in assistant metadata if not present, and is wired into seed_bundled_defaults. Legacy cleanup is simplified to remove only the older professor-mari entry.
Seeding validation tests
src-tauri/src/seed_defaults.rs
Test harness validates that seed_bundled_defaults creates the Professor Mari character under PROFESSOR_MARI_ID with correct metadata: name = "Professor Mari" and extensions.isBuiltInAssistant = true.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes


Possibly related PRs

  • Pasta-Devs/Marinara-Engine#388: Adds UI/seed-message warnings that trigger when the Professor Mari character ID is present, complementing this PR's seeding of that character.

Suggested labels

server


Poem

Behold! A new subject awakens in the vault—
Professor Mari, bound to eternal call,
Legacy dust swept clean, the blueprint refined,
Tests stand ready to guard what we have designed. ✨📚

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Fix default conversation character seed' directly addresses the main change: seeding the Professor Mari character during initialization to fix a setup blocker.
Description check ✅ Passed The description provides comprehensive context including problem statement, solution summary, proof with before/after images, and validation steps, though some validation checkboxes remain unticked.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/refactor-pr-30-default-character-seed

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added the bugfix Bug fix label May 24, 2026
@munimunigamer
Copy link
Copy Markdown
Collaborator Author

This one might be closeable or gotta figure out how we handle this since Mari is not a character no more, but an actual agent

@munimunigamer
Copy link
Copy Markdown
Collaborator Author

But needs to be changed since the seed no longer works (Mari in convo mode can't edit things hehe)

@cha1latte cha1latte self-assigned this May 24, 2026
@cha1latte
Copy link
Copy Markdown
Collaborator

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 25, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot added the server label May 25, 2026
@github-actions github-actions Bot removed the server label May 25, 2026
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
src-tauri/src/seed_defaults.rs (1)

317-340: ⚡ Quick win

Add the regression test for preserving __professor_mari__ during startup seeding.

This test proves creation, but it does not lock down the behavior changed in remove_legacy_professor_mari. A case that pre-seeds both ids, runs seed_bundled_defaults, and asserts the canonical row survives while "professor-mari" is removed would guard the actual fix.

Possible test shape
     #[test]
     fn seeds_professor_mari_as_default_character() {
         let (storage, root) = temp_storage();

         seed_bundled_defaults(&storage, &root.0.join("missing-default-data"))
             .expect("defaults should seed");

         let character = storage
             .get("characters", PROFESSOR_MARI_ID)
             .expect("character lookup should succeed")
             .expect("Professor Mari should be seeded");
         let data = character
             .get("data")
             .and_then(Value::as_str)
             .and_then(|raw| serde_json::from_str::<Value>(raw).ok())
             .expect("character data should be stored as JSON");

         assert_eq!(data["name"], "Professor Mari");
         assert_eq!(
             data.pointer("/extensions/isBuiltInAssistant")
                 .and_then(Value::as_bool),
             Some(true)
         );
     }
+
+    #[test]
+    fn preserves_canonical_professor_mari_while_removing_legacy_id() {
+        let (storage, root) = temp_storage();
+
+        seed_professor_mari_character(&storage).expect("canonical seed should succeed");
+        storage
+            .create(
+                "characters",
+                json!({
+                    "id": "professor-mari",
+                    "data": "{}",
+                    "comment": "",
+                    "avatarPath": Value::Null
+                }),
+            )
+            .expect("legacy row should be inserted");
+
+        seed_bundled_defaults(&storage, &root.0.join("missing-default-data"))
+            .expect("defaults should seed");
+
+        assert!(storage
+            .get("characters", PROFESSOR_MARI_ID)
+            .expect("canonical lookup should succeed")
+            .is_some());
+        assert!(storage
+            .get("characters", "professor-mari")
+            .expect("legacy lookup should succeed")
+            .is_none());
+    }
🤖 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 `@src-tauri/src/seed_defaults.rs` around lines 317 - 340, Add a regression test
that pre-seeds both legacy and canonical rows, runs seed_bundled_defaults, and
verifies the canonical PROFESSOR_MARI_ID row survives while the legacy
"professor-mari" row is removed: create storage via temp_storage(), insert two
character rows (one with PROFESSOR_MARI_ID and one with "professor-mari")
including appropriate "data" JSON, call seed_bundled_defaults(&storage, ...),
then use storage.get("characters", PROFESSOR_MARI_ID) and
storage.get("characters", "professor-mari") to assert the canonical row remains
and the legacy row is gone; this ensures the behavior fixed in
remove_legacy_professor_mari is regression-tested.
🤖 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 `@src-tauri/src/seed_defaults.rs`:
- Around line 12-16: In seed_bundled_defaults, reorder the operations so you
create the canonical Professor Mari before deleting the legacy row: call
seed_professor_mari_character(storage)? prior to
remove_legacy_professor_mari(storage)? (keep seed_marinara_preset(storage,
&db_root)? after both as before). This ensures if seed_professor_mari_character
fails the legacy record remains; adjust the call order in the
seed_bundled_defaults function accordingly and preserve existing ? error
propagation.

---

Nitpick comments:
In `@src-tauri/src/seed_defaults.rs`:
- Around line 317-340: Add a regression test that pre-seeds both legacy and
canonical rows, runs seed_bundled_defaults, and verifies the canonical
PROFESSOR_MARI_ID row survives while the legacy "professor-mari" row is removed:
create storage via temp_storage(), insert two character rows (one with
PROFESSOR_MARI_ID and one with "professor-mari") including appropriate "data"
JSON, call seed_bundled_defaults(&storage, ...), then use
storage.get("characters", PROFESSOR_MARI_ID) and storage.get("characters",
"professor-mari") to assert the canonical row remains and the legacy row is
gone; this ensures the behavior fixed in remove_legacy_professor_mari is
regression-tested.
🪄 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 Plus

Run ID: ccc89562-3a73-47e1-86ea-a737ed7ccaf9

📥 Commits

Reviewing files that changed from the base of the PR and between 99c21dc and 2ef16e8.

📒 Files selected for processing (1)
  • src-tauri/src/seed_defaults.rs

Comment thread src-tauri/src/seed_defaults.rs
@cha1latte
Copy link
Copy Markdown
Collaborator

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 25, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@cha1latte
Copy link
Copy Markdown
Collaborator

@munimunigamer Handled this by keeping Mari’s top-bar/agent role separate from the Conversation starter behavior.

This PR does not restore the old editable Professor Mari assistant chat or give the seeded character special agent powers. It only seeds a lightweight built-in __professor_mari__ character so fresh Conversation setup still has a selectable default and can get past the “no characters selected” blocker. The seeded row is marked with extensions.isBuiltInAssistant = true, so schedule generation skips her instead of treating her like a normal autonomous character.

I also updated the startup order so the canonical row is created before the old professor-mari legacy row is removed, and added a regression test proving the canonical row survives while the legacy one is cleaned up.

@cha1latte cha1latte merged commit a371d53 into refactor May 25, 2026
6 checks passed
@cha1latte cha1latte deleted the fix/refactor-pr-30-default-character-seed branch May 25, 2026 22:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants