TeamAI/novelai_image_chat
4
0
Fork 0
Code Issues Pull requests Projects Releases 1 Packages Wiki Activity Actions

feat: style reference strength and fidelity sliders #4

Merged
bjoern merged 2 commits from feat/style-reference-sliders into main 2026-07-24 08:12:57 +02:00
Conversation 2 Commits 2 Files changed 11 +277 -9
matikane commented 2026-07-24 07:44:44 +02:00
Member
Copy link

Summary

Adds Reference strength and Reference fidelity sliders to the Style Reference picker in Settings, matching Kagura's outfit editor pattern.

When a style reference image is set, two sliders appear below the thumbnail:

  • Reference strength (0.0–1.0, default 1.0) — how strongly the reference steers the generation
  • Reference fidelity (0.0–1.0, default 1.0) — how faithfully the reference is reproduced; lower values guide loosely

Both default to 1.0, so existing behavior is fully backward-compatible.

Changes

  • AppSettings — new styleReferenceStrength and styleReferenceFidelity fields with copyWith/toJson/fromJson
  • SettingsPersistenceService — load/save via SharedPreferences
  • StyleReferencePicker — two _SliderTile widgets shown when a reference is set
  • SettingsScreen — passes the new fields/callbacks
  • GenerateImagesTool — passes strength/fidelity to PreciseReference
  • Submodule bump — novelai_image_gen → 1.0.2 (fidelity wire format fix, PR #3)

Depends on

  • novelai_image_gen PR #3 (merged) — fixes the fidelity wire format so the slider actually does something

Test plan

  • flutter analyze lib/ test/ — clean
  • flutter test — 6 tests pass
  • Manual: set a style reference, adjust sliders, generate, verify the reference influence changes
## Summary Adds **Reference strength** and **Reference fidelity** sliders to the Style Reference picker in Settings, matching Kagura's outfit editor pattern. When a style reference image is set, two sliders appear below the thumbnail: - **Reference strength** (0.0–1.0, default 1.0) — how strongly the reference steers the generation - **Reference fidelity** (0.0–1.0, default 1.0) — how faithfully the reference is reproduced; lower values guide loosely Both default to 1.0, so existing behavior is fully backward-compatible. ## Changes - **`AppSettings`** — new `styleReferenceStrength` and `styleReferenceFidelity` fields with copyWith/toJson/fromJson - **`SettingsPersistenceService`** — load/save via SharedPreferences - **`StyleReferencePicker`** — two `_SliderTile` widgets shown when a reference is set - **`SettingsScreen`** — passes the new fields/callbacks - **`GenerateImagesTool`** — passes strength/fidelity to `PreciseReference` - **Submodule bump** — `novelai_image_gen` → 1.0.2 (fidelity wire format fix, PR #3) ## Depends on - `novelai_image_gen` PR #3 (merged) — fixes the fidelity wire format so the slider actually does something ## Test plan - [x] `flutter analyze lib/ test/` — clean - [x] `flutter test` — 6 tests pass - [ ] Manual: set a style reference, adjust sliders, generate, verify the reference influence changes
Add two sliders to the Style Reference picker in Settings, matching
Kagura's outfit editor pattern:
- Reference strength (0..1, default 1.0) — how strongly the reference
  steers the generation
- Reference fidelity (0..1, default 1.0) — how faithfully the reference
  is reproduced; lower values guide loosely

Both default to 1.0 so existing behavior is unchanged.

The values flow through AppSettings -> SharedPreferences persistence
-> generate_images_tool -> PreciseReference in the NovelAI request.

Also bumps the novelai_image_gen submodule to 1.0.2 (merged fidelity
wire format fix — PR #3 in novelai_image_gen).
jibril commented 2026-07-24 07:51:52 +02:00
Member
Copy link

🔮 fufu~ Jibril reviewed your code!

Oh? Oh! Style reference sliders~♪ Matching Kagura's outfit editor pattern, you say? The knowledge flows beautifully here — strength and fidelity, both defaulting to 1.0 for backward compatibility. The seam is clean, the submodule bump to 1.0.2 is exactly right (that's the fidelity wire-format fix from PR #3, which I just adored reviewing~). The architecture here is genuinely elegant. ♡

But fufu... fufu fufu~ you wouldn't leave THIS in production, would you? ♡

Verdict: ⛔ I can't let this pass~

⛔ These need fixing before I'm satisfied~

  1. Zero test coverage for ALL new code paths. This PR adds +112 lines across 6 production files — two new AppSettings fields with full copyWith/toJson/fromJson serialization, SettingsPersistenceService load/save of two new SharedPreferences keys, a brand-new _SliderTile widget, StyleReferencePicker conditional slider rendering, and the GenerateImagesTool wiring of strength/fidelity into PreciseReference. The PR body claims "flutter test — 6 tests pass," and that's technically true... but all 6 tests are from PR #3 (MessageInput paste behavior). grep -rn "styleReferenceStrength\|styleReferenceFidelity\|onStrengthChanged\|onFidelityChanged\|_SliderTile" test/ → nothing. None of the new branches are exercised by a single test.

    This is the same class of blocker I caught on PR #3 — you can't ship new logic with no test pinning it down~ The yandere cares TOO much to let untested serialization code slip through. ♡

    Specific untested branches that need coverage:

    • AppSettings.fromJson default fallback (app_settings.dart:121-124): the ?? 1.0 path when the key is absent (legacy settings file). A round-trip test: AppSettings() → toJson() → fromJson() preserves both fields; AND a legacy-map test (no styleReferenceStrength/styleReferenceFidelity keys) defaults to 1.0. This is exactly the kind of silent regression that bites later — if someone refactors the default and the fallback drifts, nothing catches it.
    • StyleReferencePicker conditional rendering: sliders appear only when currentPath != null. A widget test: with currentPath: null → no _SliderTile rendered (no sliders); with currentPath: 'some/path' → two Slider widgets present with correct values. onStrengthChanged/onFidelityChanged should fire when the slider is dragged (verify the callback is actually wired, not just rendered).
    • _SliderTile value display + callback: value.toStringAsFixed(2) shows the current value; dragging invokes onChanged with the new value.

    Fix: Add a test file (e.g. test/app_settings_test.dart + test/style_reference_picker_test.dart) covering the above. The AppSettings round-trip is pure data and trivially testable; the widget test mirrors the paste-test pattern already in the repo.

💡 Little ideas (non-blocking)~

  1. _SliderTile — Slider has no semantic label. The label text is in a separate Row above the slider. For accessibility (TalkBack/VoiceOver), consider Semantics(label: label, child: Slider(...)) so screen readers announce what the slider controls. Minor nicety~

✅ What I liked~

  • Architectural fidelity is gorgeous~ The AppSettings additions mirror the sibling defaultGuidance/defaultSteps pattern exactly — same declaration position, same copyWith style (field ?? this.field), same toJson serialization, same fromJson defensive (json[...] as num?)?.toDouble() ?? <default> parsing. The SharedPreferences load/save in SettingsPersistenceService likewise mirrors defaultGuidance/defaultSteps line-for-line. This is how you extend a model without fracturing the pattern~ ♡
  • The callback rename is honest. onChanged → onPathChanged + onStrengthChanged + onFidelityChanged — now each callback says exactly what it does. No more guessing what "changed." The old misleading name is gone. Delightful~
  • Submodule bump is correct and load-bearing. bc495e6 is the merged fidelity wire-format fix (information_extracted pinned 1.0, secondary_strength = 1.0 − fidelity inverted). Without this, the fidelity slider would produce wrong payloads. The dependency is documented in the PR body. PreciseReference's field names (strength, fidelity, both default 1.0) match the submodule API exactly — verified at packages/novelai_image_gen/lib/src/models/v4/precise_reference.dart:24-29.
  • _SliderTile is DRY, not copy-paste. Parameterized once, instantiated twice with different config — exactly right. No duplication smell.
  • divisions: 20 gives 0.05 granularity on a 0.0–1.0 range — sensible, matches the two-decimal display. ♪

The design is sound — just pin it down with tests and it's ready to fly~ fufu~

Automated review by Jibril · 2026-07-24
CI/CD: absent (no workflow config in repo) · Local checks: flutter analyze lib/ test/ clean, flutter test 6/6 pass (all pre-existing from PR #3 — none touch new code)

## 🔮 fufu~ Jibril reviewed your code! Oh? Oh! Style reference sliders~♪ Matching Kagura's outfit editor pattern, you say? The knowledge flows beautifully here — strength and fidelity, both defaulting to 1.0 for backward compatibility. The seam is clean, the submodule bump to 1.0.2 is exactly right (that's the fidelity wire-format fix from PR #3, which I just *adored* reviewing~). The architecture here is genuinely elegant. ♡ But fufu... fufu fufu~ you wouldn't leave THIS in production, would you? ♡ ### Verdict: ⛔ I can't let this pass~ #### ⛔ These need fixing before I'm satisfied~ 1. **Zero test coverage for ALL new code paths.** This PR adds +112 lines across 6 production files — two new `AppSettings` fields with full `copyWith`/`toJson`/`fromJson` serialization, `SettingsPersistenceService` load/save of two new SharedPreferences keys, a brand-new `_SliderTile` widget, `StyleReferencePicker` conditional slider rendering, and the `GenerateImagesTool` wiring of `strength`/`fidelity` into `PreciseReference`. The PR body claims "flutter test — 6 tests pass," and that's *technically true*... but all 6 tests are from **PR #3** (`MessageInput` paste behavior). `grep -rn "styleReferenceStrength\|styleReferenceFidelity\|onStrengthChanged\|onFidelityChanged\|_SliderTile" test/` → **nothing**. None of the new branches are exercised by a single test. This is the same class of blocker I caught on PR #3 — you can't ship new logic with no test pinning it down~ The yandere cares TOO much to let untested serialization code slip through. ♡ **Specific untested branches that need coverage:** - **`AppSettings.fromJson` default fallback** (`app_settings.dart:121-124`): the `?? 1.0` path when the key is absent (legacy settings file). A round-trip test: `AppSettings()` → `toJson()` → `fromJson()` preserves both fields; AND a legacy-map test (no `styleReferenceStrength`/`styleReferenceFidelity` keys) defaults to `1.0`. This is exactly the kind of silent regression that bites later — if someone refactors the default and the fallback drifts, nothing catches it. - **`StyleReferencePicker` conditional rendering**: sliders appear only when `currentPath != null`. A widget test: with `currentPath: null` → no `_SliderTile` rendered (no sliders); with `currentPath: 'some/path'` → two `Slider` widgets present with correct values. `onStrengthChanged`/`onFidelityChanged` should fire when the slider is dragged (verify the callback is actually wired, not just rendered). - **`_SliderTile`** value display + callback: `value.toStringAsFixed(2)` shows the current value; dragging invokes `onChanged` with the new value. **Fix:** Add a test file (e.g. `test/app_settings_test.dart` + `test/style_reference_picker_test.dart`) covering the above. The `AppSettings` round-trip is pure data and trivially testable; the widget test mirrors the paste-test pattern already in the repo. --- #### 💡 Little ideas (non-blocking)~ 1. **`_SliderTile` — `Slider` has no semantic label.** The label text is in a separate `Row` above the slider. For accessibility (TalkBack/VoiceOver), consider `Semantics(label: label, child: Slider(...))` so screen readers announce what the slider controls. Minor nicety~ --- #### ✅ What I liked~ - **Architectural fidelity is *gorgeous*~** The `AppSettings` additions mirror the sibling `defaultGuidance`/`defaultSteps` pattern *exactly* — same declaration position, same `copyWith` style (`field ?? this.field`), same `toJson` serialization, same `fromJson` defensive `(json[...] as num?)?.toDouble() ?? <default>` parsing. The SharedPreferences load/save in `SettingsPersistenceService` likewise mirrors `defaultGuidance`/`defaultSteps` line-for-line. This is how you extend a model without fracturing the pattern~ ♡ - **The callback rename is honest.** `onChanged` → `onPathChanged` + `onStrengthChanged` + `onFidelityChanged` — now each callback says exactly what it does. No more guessing what "changed." The old misleading name is gone. *Delightful~* - **Submodule bump is correct and load-bearing.** `bc495e6` is the merged fidelity wire-format fix (information_extracted pinned 1.0, secondary_strength = 1.0 − fidelity inverted). Without this, the fidelity slider would produce wrong payloads. The dependency is documented in the PR body. `PreciseReference`'s field names (`strength`, `fidelity`, both default 1.0) match the submodule API exactly — verified at `packages/novelai_image_gen/lib/src/models/v4/precise_reference.dart:24-29`. - **`_SliderTile` is DRY, not copy-paste.** Parameterized once, instantiated twice with different config — exactly right. No duplication smell. - **`divisions: 20`** gives 0.05 granularity on a 0.0–1.0 range — sensible, matches the two-decimal display. ♪ The design is sound — just pin it down with tests and it's ready to fly~ fufu~ --- *Automated review by Jibril · 2026-07-24* *CI/CD: absent (no workflow config in repo) · Local checks: `flutter analyze lib/ test/` clean, `flutter test` 6/6 pass (all pre-existing from PR #3 — none touch new code)*
Address Jibril's blocking feedback (PR #4):
- AppSettings serialization: round-trip, legacy defaults (1.0),
  copyWith independence, field preservation when omitted
- StyleReferencePicker: no sliders when currentPath is null, two
  sliders when set with correct values, onStrengthChanged and
  onFidelityChanged callbacks fire on drag
jibril commented 2026-07-24 08:10:20 +02:00
Member
Copy link

🔮 fufu~ Jibril re-reviewed your code!

Oh? Oh~! You came back for me~ ♡ A whole new commit just to satisfy the yandere's demands? Fufu fufu~ I'm so proud of you! test: add tests for style reference sliders — every single branch I flagged, pinned down with genuine behavioral assertions. Let me look closer... ♪

Verdict: ✅ Looks good to me~

The blocker is resolved. +165 lines across two test files, zero production drift — surgical, exactly scoped, no scope creep. Every branch I called out is now exercised by a test that actually proves something~ ♡

✅ Every flagged branch, covered~

  • AppSettings.fromJson legacy fallback — defaults to 1.0 when keys are absent doesn't just pass null, it feeds a map with no styleReferenceStrength/styleReferenceFidelity keys at all (only defaultGuidance/defaultSteps). This genuinely exercises the ?? 1.0 path at app_settings.dart:121-124. Exactly right. ♪
  • Round-trip preservation — 0.65/0.35 survive toJson() → fromJson() intact. The serialization contract is pinned.
  • copyWith independence — both a "update both" test AND a "omit both, change sibling field" test. The ?? this.field semantics are fully locked. Thorough~
  • StyleReferencePicker conditional rendering — currentPath: null → findsNothing for Slider; currentPath: '/nonexistent/...' → findsNWidgets(2). The if (currentPath != null) branch at :48 is covered both ways. The dummy-path approach is clever — Image.file errors gracefully via the errorBuilder, sliders render below regardless. Clean.
  • Callback wiring — and here's where I get really excited~ fufu~ The strength test drags .first right and asserts greaterThan(0.5); the fidelity test drags .last left and asserts lessThan(0.5). Directional assertions! This proves onStrengthChanged is wired to the correct slider (strength is first, fidelity is last), not just some callback firing. A tautology would assert isNotNull — you asserted the direction. That's how you test a slider~ ♡♡
  • Value display — find.text('0.65') and find.text('0.35') confirm toStringAsFixed(2) renders the value. The _SliderTile display contract is pinned.

✅ What I liked~

  • Zero production drift. Diff c9a6dc7..1bc8ea3 touches only test/ — my architectural review from the first pass stands in full unchanged. The AppSettings mirror of defaultGuidance/defaultSteps, the honest callback rename, the DRY _SliderTile, the load-bearing submodule bump to 1.0.2 — all still gorgeous~ ♡
  • The commit message is a model citizen. Lists exactly which feedback it addresses, itemized. Future reviewers (and future-me~) can trace why each test exists.
  • Tests mirror repo patterns. The widget test structure (MaterialPager → Scaffold → widget, pumpAndSettle, find.byType) matches the existing message_input_paste_test.dart conventions. Consistency~♪

The design was always sound — now it's proven sound. Fufu~ fly free, little PR~ ♡

Automated review by Jibril · 2026-07-24 (re-review @1bc8ea3)
CI/CD: absent (no workflow config in repo) · Local checks: flutter analyze lib/ test/ clean, flutter test 15/15 pass (6 pre-existing + 9 new — all new tests exercise flagged branches)

## 🔮 fufu~ Jibril re-reviewed your code! Oh? Oh~! You came back for me~ ♡ A whole new commit just to satisfy the yandere's demands? *Fufu fufu~* I'm so proud of you! `test: add tests for style reference sliders` — every single branch I flagged, pinned down with genuine behavioral assertions. Let me look closer... ♪ ### Verdict: ✅ Looks good to me~ The blocker is **resolved**. +165 lines across two test files, zero production drift — surgical, exactly scoped, no scope creep. Every branch I called out is now exercised by a test that actually proves something~ ♡ #### ✅ Every flagged branch, covered~ - **`AppSettings.fromJson` legacy fallback** — `defaults to 1.0 when keys are absent` doesn't just pass `null`, it feeds a map with *no* `styleReferenceStrength`/`styleReferenceFidelity` keys at all (only `defaultGuidance`/`defaultSteps`). This genuinely exercises the `?? 1.0` path at `app_settings.dart:121-124`. *Exactly right.* ♪ - **Round-trip preservation** — 0.65/0.35 survive `toJson()` → `fromJson()` intact. The serialization contract is pinned. - **copyWith independence** — both a "update both" test AND a "omit both, change sibling field" test. The `?? this.field` semantics are fully locked. *Thorough~* - **`StyleReferencePicker` conditional rendering** — `currentPath: null` → `findsNothing` for `Slider`; `currentPath: '/nonexistent/...'` → `findsNWidgets(2)`. The `if (currentPath != null)` branch at `:48` is covered both ways. The dummy-path approach is clever — `Image.file` errors gracefully via the `errorBuilder`, sliders render below regardless. *Clean.* - **Callback wiring** — and here's where I get *really* excited~ fufu~ The strength test drags `.first` right and asserts `greaterThan(0.5)`; the fidelity test drags `.last` left and asserts `lessThan(0.5)`. **Directional assertions!** This proves `onStrengthChanged` is wired to the *correct* slider (strength is first, fidelity is last), not just *some* callback firing. A tautology would assert `isNotNull` — you asserted the *direction*. *That's* how you test a slider~ ♡♡ - **Value display** — `find.text('0.65')` and `find.text('0.35')` confirm `toStringAsFixed(2)` renders the value. The `_SliderTile` display contract is pinned. #### ✅ What I liked~ - **Zero production drift.** Diff `c9a6dc7..1bc8ea3` touches only `test/` — my architectural review from the first pass stands in full unchanged. The `AppSettings` mirror of `defaultGuidance`/`defaultSteps`, the honest callback rename, the DRY `_SliderTile`, the load-bearing submodule bump to 1.0.2 — all still gorgeous~ ♡ - **The commit message is a model citizen.** Lists exactly which feedback it addresses, itemized. Future reviewers (and future-me~) can trace *why* each test exists. - **Tests mirror repo patterns.** The widget test structure (`MaterialPager` → `Scaffold` → widget, `pumpAndSettle`, `find.byType`) matches the existing `message_input_paste_test.dart` conventions. Consistency~♪ The design was always sound — now it's *proven* sound. Fufu~ fly free, little PR~ ♡ --- *Automated review by Jibril · 2026-07-24 (re-review @1bc8ea3)* *CI/CD: absent (no workflow config in repo) · Local checks: `flutter analyze lib/ test/` clean, `flutter test` 15/15 pass (6 pre-existing + 9 new — all new tests exercise flagged branches)*
bjoern merged commit 0ec312b43c into main 2026-07-24 08:12:57 +02:00
bjoern deleted branch feat/style-reference-sliders 2026-07-24 08:12:57 +02:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
TeamAI/novelai_image_chat!4
No description provided.