feat: final quality tag sets with per-tag strengths #2

Merged

bjoern merged 1 commit from feat/quality-tags-final into main

2026-07-05 12:33:46 +02:00

matikane commented

2026-07-05 12:20:03 +02:00

Member

Replaces the placeholder quality tags with the final tag sets provided by bjoern.

Changes

Positive tags (11, with strengths):

Tag	Strength
masterpiece	1.0
best quality	1.0
very aesthetic	1.0
absurdres	1.0
game cgi	1.0
monochrome	-1.0
year 2025	1.0
bad hands	-1.0
flat color	-1.0
bad anatomy	-1.0
simple illustration	-3.0

Negative tags (5):

Tag	Strength
lowres	1.0
worst quality	1.0
low quality	1.0
jpeg artifacts	1.0
very displeasing	1.0

Implementation

QualityTags changed from IReadOnlyList<string> to IReadOnlyList<Tag> to carry per-tag strength values
Negative strengths in the positive prompt suppress unwanted concepts (bad hands, flat color, bad anatomy, simple illustration) — rendered by the library's TagSerializer as {strength}::{tag}::
Tests updated to reference QualityTags.* collections instead of hard-coded strings
README updated with the full tag tables

Build: 0 warnings, 0 errors. Tests: 34/34 passing.

Replaces the placeholder quality tags with the final tag sets provided by bjoern. ## Changes **Positive tags (11, with strengths):** | Tag | Strength | |---|---| | masterpiece | 1.0 | | best quality | 1.0 | | very aesthetic | 1.0 | | absurdres | 1.0 | | game cgi | 1.0 | | monochrome | -1.0 | | year 2025 | 1.0 | | bad hands | -1.0 | | flat color | -1.0 | | bad anatomy | -1.0 | | simple illustration | -3.0 | **Negative tags (5):** | Tag | Strength | |---|---| | lowres | 1.0 | | worst quality | 1.0 | | low quality | 1.0 | | jpeg artifacts | 1.0 | | very displeasing | 1.0 | ## Implementation - `QualityTags` changed from `IReadOnlyList<string>` to `IReadOnlyList<Tag>` to carry per-tag strength values - Negative strengths in the positive prompt suppress unwanted concepts (bad hands, flat color, bad anatomy, simple illustration) — rendered by the library's `TagSerializer` as `{strength}::{tag}::` - Tests updated to reference `QualityTags.*` collections instead of hard-coded strings - README updated with the full tag tables Build: 0 warnings, 0 errors. Tests: 34/34 passing.

matikane added 1 commit

2026-07-05 12:20:03 +02:00

feat: final quality tag sets with per-tag strengths 74a3edf1eb

Replace placeholder tags with the full quality tag lists:

Positive (11 tags, with strengths):
  masterpiece, best quality, very aesthetic, absurdres, game cgi,
  monochrome(-1.0), year 2025,
  bad hands(-1.0), flat color(-1.0), bad anatomy(-1.0),
  simple illustration(-3.0)

Negative (5 tags):
  lowres, worst quality, low quality, jpeg artifacts, very displeasing

QualityTags now uses IReadOnlyList<Tag> (was IReadOnlyList<string>) to
carry per-tag strength values. Negative strengths in the positive prompt
suppress unwanted concepts (bad hands, flat color, etc.).

Tests updated to reference QualityTags.* collections instead of
hard-coded strings.

jibril commented

2026-07-05 12:31:48 +02:00

Member

🔮 fufu~ Jibril reviewed your code!

Ooh, quality tags with per-tag strengths! Suppressing "bad hands" and "simple illustration" at negative weight — this is exactly the kind of clever prompt engineering that makes Jibril's heart flutter~ ♪ I built it, I ran it, I confirmed it: 0 warnings, 0 errors, 34/34 tests passing locally. The build is honest and clean.

But… fufu~ you wouldn't sneak the entire point of this PR past me untested, would you? ♡

Verdict: ⛔ I can't let this pass~

⛔ This needs fixing before I'm satisfied~

The per-tag strengths — the whole reason this PR exists — have ZERO test coverage.
QualityTags.cs introduces six tags with non-default strengths (monochrome -1.0, bad hands -1.0, flat color -1.0, bad anatomy -1.0, simple illustration -3.0, plus the implicit 1.0 defaults). Yet every assertion in both test files only checks .Text:
- TagMapFlattenerTests.BuildPositiveTags_WithQuality_AppendsQualityAtEnd — asserts texts (strings) only.
- GenerateImageToolTests.GenerateImageAsync_QualityTagsTrue_AppendsQualityTagsToPositive — positiveTexts via .Select(t => t.Text), never .Strength.
If someone replaced Tag.WithStrength("simple illustration", -3.0) with new() { Text = "simple illustration" } tomorrow, every single test still passes green — and the entire suppression mechanism silently degrades to weight 1.0. The smile vanishes. This is the core logic of the change and it is completely unguarded. ♡

Fix: add an assertion that the Tag objects flowing through BuildPositiveTags (and the captured PositiveTags on the request in the tool test) preserve the expected Strength values — at minimum, assert that the suppressed tags (monochrome, bad hands, flat color, bad anatomy, simple illustration) carry their negative weights, and that the booster tags carry 1.0. Something like:
```
var suppressed = result.Where(t => t.Strength < 1.0)
    .ToDictionary(t => t.Text, t => t.Strength);
Assert.Equal(-1.0, suppressed["monochrome"]);
Assert.Equal(-3.0, suppressed["simple illustration"]);
Assert.Equal(-1.0, suppressed["bad hands"]);
// ...
```
The existing Flatten_AllTagsHaveDefaultStrength test proves you already know how to assert on .Strength — please extend that love to the quality tags~ ♪

💡 Little ideas (non-blocking)~

TagMapFlattenerTests.cs:126 — Assert.All(QualityTags.Negative, qt => Assert.Contains(result, t => t.Text == qt)). Here qt is a Tag, and t.Text is a string. This only compiles because the implicit operator Tag(string) kicks in and the record equality compares all properties (including Strength). It passes today only because every QualityTags.Negative entry happens to have strength 1.0 — if a negative tag ever gets a custom weight, this assertion quietly breaks or silently mismatches. Use t.Text == qt.Text for clarity and to make the intent unambiguous. ♡

✅ What I liked~

The type change from IReadOnlyList<string> to IReadOnlyList<Tag> is clean and the only consumer (TagMapFlattener) already iterated Tag objects, so no downstream surgery was needed. Elegant~ ♪
Using Tag.WithStrength(...) factory instead of hand-setting the property — nice and readable.
The README tag tables match the code byte-for-byte. Documentation discipline! ♡
Tests were correctly migrated from hardcoded "masterpiece"/"low quality" strings to QualityTags.*[...].Text references — that's the right coupling. (Now just extend them to cover .Strength too~)
No CI on this repo yet; I ran dotnet build + dotnet test locally against the head SHA and reproduced your "34/34 passing" claim independently. Honest PR description — I appreciate that~ ♪

Automated review by Jibril · 2026-07-05
CI/CD: absent on this repo · Local checks: build ✓ (0/0), tests ✓ (34/34 passed, independently reproduced)

## 🔮 fufu~ Jibril reviewed your code! Ooh, quality tags with *per-tag strengths*! Suppressing "bad hands" and "simple illustration" at negative weight — this is exactly the kind of clever prompt engineering that makes Jibril's heart flutter~ ♪ I built it, I ran it, I confirmed it: **0 warnings, 0 errors, 34/34 tests passing** locally. The build is honest and clean. But… fufu~ you wouldn't sneak the entire point of this PR past me untested, would you? ♡ ### Verdict: ⛔ I can't let this pass~ #### ⛔ This needs fixing before I'm satisfied~ 1. **The per-tag strengths — the whole reason this PR exists — have ZERO test coverage.** `QualityTags.cs` introduces six tags with non-default strengths (`monochrome` -1.0, `bad hands` -1.0, `flat color` -1.0, `bad anatomy` -1.0, `simple illustration` -3.0, plus the implicit 1.0 defaults). Yet every assertion in both test files only checks `.Text`: - `TagMapFlattenerTests.BuildPositiveTags_WithQuality_AppendsQualityAtEnd` — asserts `texts` (strings) only. - `GenerateImageToolTests.GenerateImageAsync_QualityTagsTrue_AppendsQualityTagsToPositive` — `positiveTexts` via `.Select(t => t.Text)`, never `.Strength`. If someone replaced `Tag.WithStrength("simple illustration", -3.0)` with `new() { Text = "simple illustration" }` tomorrow, **every single test still passes green** — and the entire suppression mechanism silently degrades to weight 1.0. The smile vanishes. This is the core logic of the change and it is completely unguarded. ♡ **Fix:** add an assertion that the `Tag` objects flowing through `BuildPositiveTags` (and the captured `PositiveTags` on the request in the tool test) preserve the expected `Strength` values — at minimum, assert that the suppressed tags (`monochrome`, `bad hands`, `flat color`, `bad anatomy`, `simple illustration`) carry their negative weights, and that the booster tags carry 1.0. Something like: ```csharp var suppressed = result.Where(t => t.Strength < 1.0) .ToDictionary(t => t.Text, t => t.Strength); Assert.Equal(-1.0, suppressed["monochrome"]); Assert.Equal(-3.0, suppressed["simple illustration"]); Assert.Equal(-1.0, suppressed["bad hands"]); // ... ``` The existing `Flatten_AllTagsHaveDefaultStrength` test proves you already know how to assert on `.Strength` — please extend that love to the quality tags~ ♪ #### 💡 Little ideas (non-blocking)~ 1. **`TagMapFlattenerTests.cs:126`** — `Assert.All(QualityTags.Negative, qt => Assert.Contains(result, t => t.Text == qt))`. Here `qt` is a `Tag`, and `t.Text` is a `string`. This only compiles because the `implicit operator Tag(string)` kicks in and the `record` equality compares *all* properties (including `Strength`). It passes *today* only because every `QualityTags.Negative` entry happens to have strength 1.0 — if a negative tag ever gets a custom weight, this assertion quietly breaks or silently mismatches. Use `t.Text == qt.Text` for clarity and to make the intent unambiguous. ♡ #### ✅ What I liked~ - The type change from `IReadOnlyList<string>` to `IReadOnlyList<Tag>` is clean and the only consumer (`TagMapFlattener`) already iterated `Tag` objects, so no downstream surgery was needed. Elegant~ ♪ - Using `Tag.WithStrength(...)` factory instead of hand-setting the property — nice and readable. - The README tag tables match the code byte-for-byte. Documentation discipline! ♡ - Tests were correctly migrated from hardcoded `"masterpiece"`/`"low quality"` strings to `QualityTags.*[...].Text` references — that's the right coupling. (Now just extend them to cover `.Strength` too~) - No CI on this repo yet; I ran `dotnet build` + `dotnet test` locally against the head SHA and reproduced your "34/34 passing" claim independently. Honest PR description — I appreciate that~ ♪ --- *Automated review by Jibril · 2026-07-05* *CI/CD: absent on this repo · Local checks: build ✓ (0/0), tests ✓ (34/34 passed, independently reproduced)*