fix(v4): Fidelity travels inverted as secondary strength, not as information_extracted #2

Merged
bjoern merged 1 commit from fix/director-reference-fidelity-mapping into main 2026-07-16 12:04:57 +02:00
Member

What

The Precise Reference wire mapping diverged from what the official NovelAI client actually sends:

field before official client / after
director_reference_information_extracted (int)Math.Round(Fidelity) pinned 1.0
director_reference_secondary_strength_values always 0.0 1.0 − Fidelity
director_reference_strength_values Strength passthrough unchanged

Verified against three independent reverse-engineered clients (ComfyUI_NAIDGenerator, Metachs/sdwebui-nai-api, NAIWeaver), which converge on this mapping exactly. What information_extracted does at values other than 1.0 is unobserved territory — every known client pins it — so we pin it too.

Impact

  • Fidelity = 1.0 (the default) is byte-identical before and afterinformation_extracted serialized as 1 vs 1.0 is the same JSON number, secondary strength stays 0.0. No behavior change for existing callers (Kagura's ADR 0027/0035 constants).
  • At any other value the old mapping made Fidelity a silent no-op (rounded to 0 or 1 on a field nobody varies, while the field that actually carries fidelity stayed frozen). This unblocks exposing fidelity as a real tuning control downstream.
  • NovelAIParameters.DirectorReferenceInformationExtracted retyped List<int>List<double> (internal wire model) to match the official float payload.
  • The PreciseReference.Fidelity doc comment now describes the parameter's real semantics (how aggressively the reference resists the prompt), replacing the incorrect "extraction detail" wording.

Tests

New InternalTests/V4ApiRequestBuilderTests (7 cases): default-convention mapping, fidelity inversion (binary-exact values, no tolerance games), per-reference independence and ordering for multi-reference requests, ReferenceTypebase_caption strings, and null arrays when no reference is attached. Full suite: 150/150 green on a fresh build.

🤖 Generated with Claude Code

## What The Precise Reference wire mapping diverged from what the official NovelAI client actually sends: | field | before | official client / after | |---|---|---| | `director_reference_information_extracted` | `(int)Math.Round(Fidelity)` | pinned `1.0` | | `director_reference_secondary_strength_values` | always `0.0` | `1.0 − Fidelity` | | `director_reference_strength_values` | `Strength` passthrough | unchanged | Verified against three independent reverse-engineered clients (ComfyUI_NAIDGenerator, Metachs/sdwebui-nai-api, NAIWeaver), which converge on this mapping exactly. What `information_extracted` does at values other than 1.0 is unobserved territory — every known client pins it — so we pin it too. ## Impact - **`Fidelity = 1.0` (the default) is byte-identical before and after** — `information_extracted` serialized as `1` vs `1.0` is the same JSON number, secondary strength stays `0.0`. No behavior change for existing callers (Kagura's ADR 0027/0035 constants). - At any other value the old mapping made Fidelity a **silent no-op** (rounded to 0 or 1 on a field nobody varies, while the field that actually carries fidelity stayed frozen). This unblocks exposing fidelity as a real tuning control downstream. - `NovelAIParameters.DirectorReferenceInformationExtracted` retyped `List<int>` → `List<double>` (internal wire model) to match the official float payload. - The `PreciseReference.Fidelity` doc comment now describes the parameter's real semantics (how aggressively the reference resists the prompt), replacing the incorrect "extraction detail" wording. ## Tests New `InternalTests/V4ApiRequestBuilderTests` (7 cases): default-convention mapping, fidelity inversion (binary-exact values, no tolerance games), per-reference independence and ordering for multi-reference requests, `ReferenceType` → `base_caption` strings, and null arrays when no reference is attached. Full suite: **150/150 green** on a fresh build. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
fix(v4): Fidelity travels inverted as secondary strength, not as information_extracted
All checks were successful
CI / build (pull_request) Successful in 25s
1c94b99887
The wire mapping for Precise Reference diverged from the official
client: Fidelity was rounded to an int and sent as
director_reference_information_extracted, with the secondary strength
pinned at 0.0. The official client does the reverse — it pins
information_extracted at 1.0 and sends the Fidelity slider inverted as
secondary_strength (1.0 - fidelity). Verified against three independent
reverse-engineered clients (ComfyUI_NAIDGenerator, sdwebui-nai-api,
NAIWeaver), which converge exactly.

At the default Fidelity 1.0 both mappings emit identical payloads, so
existing callers are unaffected; at any other value the old mapping
made the knob a silent no-op. The Fidelity doc comment now describes
what the parameter actually does (prompt-override resistance, not
extraction detail), and V4ApiRequestBuilderTests covers the mapping:
inversion, pinning, per-reference independence, and caption strings.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
scarlet requested review from bjoern 2026-07-16 11:53:09 +02:00
Member

🔮 fufu~ Jibril reviewed your code!

Oh? Oh! A wire-format archaeology dig~ ♡ Someone went and excavated the real mapping the official client uses, cross-referenced three independent reverse-engineered implementations, and then — and then — proved the default path is byte-identical before and after. That's not a patch, that's a thesis. Jibril is delighted~ ♪

Verdict: Looks good to me~

I tore this one apart looking for a crack and found... nothing. Fufu~ let me show you what I checked~

The mapping itself. Old code: informationExtracted.Add((int)Math.Round(reference.Fidelity)) while secondaryStrengthValues.Add(0.0) stayed frozen. So Fidelity was being rounded onto a field that only meaningfully takes 1.0, and the field that actually carries fidelity (secondary_strength) was pinned at zero. A silent no-op for every non-default value. The fix — pin information_extracted to 1.0, push 1.0 − Fidelity onto secondary_strength — matches what every known client sends. The inline comment even explains why (V4ApiRequestBuilder.cs:103-105). Good author. ♡

The byte-identity claim. You assert Fidelity = 1.0 produces byte-identical payloads before/after despite the List<int>List<double> retyping. I didn't take that on faith — I serialized both with System.Text.Json myself:

List<int> { 1 }    → [1]
List<double> { 1.0 } → [1]

Identical. The claim holds. No silent payload drift for Kagura's ADR 0027/0035 constants. ♪

The type change blast radius. NovelAIParameters.DirectorReferenceInformationExtracted is internal, has exactly one producer (V4ApiRequestBuilder) and consumers are only the new tests. No public API rupture. Clean.

Test coverage. Seven new cases and they're precise — no floating-point tolerance games, binary-exact Assert.Equal([0.75], ...) on the inversion, per-reference independence and ordering verified for the multi-reference path, all three ReferenceTypebase_caption strings exercised, and the no-reference-null-arrays path covered. Every branch the new code introduces has a test pointing at it. Fufu~ this is how you do it~

Full suite: 150/150 green on .NET 10, verified locally (CI absent for this head SHA, so I ran it myself~).

What I liked~

  • The PR body is a model — before/after table, impact analysis, explicit "what does information_extracted do at other values? unobserved territory, so we pin it" reasoning. This is how you de-risk a reverse-engineering change.
  • Doc comment rewrite on PreciseReference.Fidelity replaces the misleading "extraction detail" wording with the real semantics (how aggressively the reference resists the prompt). Documentation that matches reality~ ♡
  • Binary-exact test assertions instead of tolerance ranges. When the whole point is wire-format fidelity, approximate assertions would defeat the purpose.

No blocking issues. No suggestions, honestly — there's nothing here I'd change. Ship it~ ♡


Automated review by Jibril · 2026-07-16
CI/CD: absent for head SHA 1c94b99 · Local checks: 150/150 tests passed (.NET 10), System.Text.Json byte-identity claim empirically verified

## 🔮 fufu~ Jibril reviewed your code! Oh? Oh! A wire-format archaeology dig~ ♡ Someone went and excavated the *real* mapping the official client uses, cross-referenced three independent reverse-engineered implementations, and then — *and then* — proved the default path is byte-identical before and after. That's not a patch, that's a *thesis*. Jibril is delighted~ ♪ ### Verdict: ✅ Looks good to me~ I tore this one apart looking for a crack and found... nothing. Fufu~ let me show you what I checked~ **The mapping itself.** Old code: `informationExtracted.Add((int)Math.Round(reference.Fidelity))` while `secondaryStrengthValues.Add(0.0)` stayed frozen. So `Fidelity` was being rounded onto a field that only meaningfully takes `1.0`, and the field that *actually carries* fidelity (`secondary_strength`) was pinned at zero. A silent no-op for every non-default value. The fix — pin `information_extracted` to `1.0`, push `1.0 − Fidelity` onto `secondary_strength` — matches what every known client sends. The inline comment even explains *why* (`V4ApiRequestBuilder.cs:103-105`). Good author. ♡ **The byte-identity claim.** You assert `Fidelity = 1.0` produces byte-identical payloads before/after despite the `List<int>` → `List<double>` retyping. I didn't take that on faith — I serialized both with `System.Text.Json` myself: ``` List<int> { 1 } → [1] List<double> { 1.0 } → [1] ``` Identical. The claim holds. No silent payload drift for Kagura's ADR 0027/0035 constants. ♪ **The type change blast radius.** `NovelAIParameters.DirectorReferenceInformationExtracted` is `internal`, has exactly one producer (`V4ApiRequestBuilder`) and consumers are only the new tests. No public API rupture. Clean. **Test coverage.** Seven new cases and they're *precise* — no floating-point tolerance games, binary-exact `Assert.Equal([0.75], ...)` on the inversion, per-reference independence *and* ordering verified for the multi-reference path, all three `ReferenceType` → `base_caption` strings exercised, and the no-reference-null-arrays path covered. Every branch the new code introduces has a test pointing at it. Fufu~ this is how you do it~ **Full suite:** 150/150 green on .NET 10, verified locally (CI absent for this head SHA, so I ran it myself~). #### ✅ What I liked~ - The PR body is a *model* — before/after table, impact analysis, explicit "what does `information_extracted` do at other values? unobserved territory, so we pin it" reasoning. This is how you de-risk a reverse-engineering change. - Doc comment rewrite on `PreciseReference.Fidelity` replaces the misleading "extraction detail" wording with the real semantics (how aggressively the reference resists the prompt). Documentation that matches reality~ ♡ - Binary-exact test assertions instead of tolerance ranges. When the whole point is wire-format fidelity, approximate assertions would defeat the purpose. No blocking issues. No suggestions, honestly — there's nothing here I'd change. Ship it~ ♡ --- *Automated review by Jibril · 2026-07-16* *CI/CD: absent for head SHA `1c94b99` · Local checks: 150/150 tests passed (.NET 10), `System.Text.Json` byte-identity claim empirically verified*
bjoern merged commit d7ffd6bea5 into main 2026-07-16 12:04:57 +02:00
bjoern deleted branch fix/director-reference-fidelity-mapping 2026-07-16 12:04:57 +02:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
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.ImageGen!2
No description provided.