Skip to content

Commit 3151b8c

Browse files
committed
gallery: remove duplicated entries and lint against them coming back
gallery/index.yaml declared eight names twice: deepseek-r1-distill-llama-8b, llama3.2-3b-enigma, qwen3-asr-0.6b, qwen3-asr-1.7b, qwopus-glm-18b-merged, voice-en-us-kathleen-low, whisper-large-q5_0 and whisper-small-q5_1. FindGalleryElement resolves a reference by returning the first match, so in every pair the second copy was unreachable: it could not be installed, could not be selected as a variant target, and could not be corrected, because any edit to it went to a copy nobody reads. A reference to such a name is also ambiguous to anything reasoning over the catalog, which is why the variant proposal job refuses to propose against them. Each pair was compared both as parsed entries and as raw text, and all eight were byte-identical apart from position. None of the sixteen blocks defines a YAML anchor or pulls one in with a merge key, so nothing was reachable only through a deleted block, and no entry named a removed copy as a variant target. Removing the second copy of each therefore changes no behaviour: the parsed set loses exactly eight entries and every surviving entry is field-for-field unchanged. The removal is textual, by line range, so the diff is pure deletions rather than a reflow of forty thousand lines. checkNoDuplicateEntryNames is the rule that keeps them out, added beside the existing gallery invariants and reporting in the same style. checkSingleVariantClaim closes the adjacent gap in the same place. VariantParents resolves a build claimed by two parents by taking the first in gallery order and calls that deterministic "for a gallery the linter would reject", but nothing rejected it: the invariant was held by curation alone. Now it is a rule, and the comment describes something real. No target is doubly claimed today, so the rule is green on arrival. Assisted-by: Claude:claude-opus-4-8 Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
1 parent 2f33ad6 commit 3151b8c

2 files changed

Lines changed: 193 additions & 212 deletions

File tree

core/gallery/variants_lint_test.go

Lines changed: 193 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,77 @@ func checkEntriesInstallSomething(entries []gallery.GalleryModel) []variantViola
116116
return violations
117117
}
118118

119+
// checkNoDuplicateEntryNames verifies no two entries share a name.
120+
//
121+
// FindGalleryElement resolves a reference by scanning and returning the first
122+
// match, so a second entry carrying an already-used name is unreachable: it can
123+
// never be installed, never be selected as a variant target, and never be
124+
// corrected, because every edit to it goes to a copy nobody reads. A reference
125+
// to such a name is also ambiguous to a reader and to any tooling that reasons
126+
// over the catalog, which is the harm even when the two copies happen to agree
127+
// today.
128+
func checkNoDuplicateEntryNames(entries []gallery.GalleryModel) []variantViolation {
129+
seen := make(map[string]struct{}, len(entries))
130+
var violations []variantViolation
131+
for _, e := range entries {
132+
if _, dup := seen[e.Name]; dup {
133+
violations = append(violations, variantViolation{
134+
Entry: e.Name,
135+
Detail: fmt.Sprintf("entry %q is declared more than once: FindGalleryElement returns the first match, "+
136+
"so only the first declaration is reachable and this one is dead weight. "+
137+
"Remove the extra copy, or rename it if the two are meant to be different models", e.Name),
138+
})
139+
continue
140+
}
141+
seen[e.Name] = struct{}{}
142+
}
143+
return violations
144+
}
145+
146+
// checkSingleVariantClaim verifies no entry is offered as a variant by more
147+
// than one parent.
148+
//
149+
// VariantParents hides a claimed build from a collapsed listing and answers
150+
// matches on it with the parent instead. With two parents claiming the same
151+
// build it picks the first in gallery order, which is deterministic but
152+
// arbitrary: the user is shown one of two equally valid rows, and which one
153+
// depends on authoring order rather than on anything about the models. That
154+
// resolution exists so the listing stays coherent for a catalog carrying the
155+
// mistake; this is the rule that keeps the catalog from carrying it.
156+
func checkSingleVariantClaim(entries []gallery.GalleryModel) []variantViolation {
157+
byName := indexEntriesByName(entries)
158+
// Only the first claim per parent counts, so a parent listing the same
159+
// target twice is a redundancy inside one stanza and not two parents
160+
// fighting over a build.
161+
claimedBy := make(map[string]string, len(entries))
162+
var violations []variantViolation
163+
for _, e := range entries {
164+
for _, v := range e.Variants {
165+
// A dangling reference and a nested one are checkVariantReferences'
166+
// business, and neither claims anything VariantParents would hide.
167+
target, ok := byName[v.Model]
168+
if !ok || target.HasVariants() || v.Model == e.Name {
169+
continue
170+
}
171+
first, claimed := claimedBy[v.Model]
172+
if !claimed {
173+
claimedBy[v.Model] = e.Name
174+
continue
175+
}
176+
if first == e.Name {
177+
continue
178+
}
179+
violations = append(violations, variantViolation{
180+
Entry: e.Name,
181+
Variant: v.Model,
182+
Detail: fmt.Sprintf("already offered as a variant by %q; a build may have only one parent, "+
183+
"otherwise which row a collapsed listing shows for it depends on authoring order", first),
184+
})
185+
}
186+
}
187+
return violations
188+
}
189+
119190
// loadGalleryIndex parses gallery/index.yaml once for the whole suite. The
120191
// index carries well over a thousand entries, so re-parsing it per spec is
121192
// pure overhead.
@@ -181,6 +252,118 @@ var _ = Describe("gallery variant lint helpers", func() {
181252

182253
Expect(checkVariantReferences(entries)).To(BeEmpty())
183254
Expect(checkEntriesInstallSomething(entries)).To(BeEmpty())
255+
Expect(checkNoDuplicateEntryNames(entries)).To(BeEmpty())
256+
Expect(checkSingleVariantClaim(entries)).To(BeEmpty())
257+
})
258+
259+
Describe("checkNoDuplicateEntryNames", func() {
260+
It("flags a second entry carrying an already-used name", func() {
261+
entries := []gallery.GalleryModel{
262+
plainEntry("twin", "u://first"),
263+
plainEntry("other", "u://other"),
264+
plainEntry("twin", "u://second"),
265+
}
266+
267+
violations := checkNoDuplicateEntryNames(entries)
268+
Expect(violations).To(HaveLen(1))
269+
Expect(violations[0].Entry).To(Equal("twin"))
270+
Expect(violations[0].Detail).To(ContainSubstring("declared more than once"))
271+
Expect(violations[0].Detail).To(ContainSubstring("only the first declaration is reachable"))
272+
})
273+
274+
// The copies in gallery/index.yaml were identical, so a rule that only
275+
// fired on differing copies would have let every one of them through.
276+
It("flags a duplicate whose fields match the first copy exactly", func() {
277+
entries := []gallery.GalleryModel{
278+
plainEntry("twin", "u://same"),
279+
plainEntry("twin", "u://same"),
280+
}
281+
282+
Expect(checkNoDuplicateEntryNames(entries)).To(HaveLen(1))
283+
})
284+
285+
It("reports every breach in one pass rather than stopping at the first", func() {
286+
entries := []gallery.GalleryModel{
287+
plainEntry("a", "u://a"),
288+
plainEntry("b", "u://b"),
289+
plainEntry("a", "u://a2"),
290+
plainEntry("b", "u://b2"),
291+
}
292+
293+
Expect(checkNoDuplicateEntryNames(entries)).To(HaveLen(2))
294+
})
295+
296+
// A name used three times costs two removals, so all the copies past
297+
// the first have to be named rather than only the second.
298+
It("flags every copy past the first", func() {
299+
entries := []gallery.GalleryModel{
300+
plainEntry("a", "u://1"),
301+
plainEntry("a", "u://2"),
302+
plainEntry("a", "u://3"),
303+
}
304+
305+
Expect(checkNoDuplicateEntryNames(entries)).To(HaveLen(2))
306+
})
307+
})
308+
309+
Describe("checkSingleVariantClaim", func() {
310+
It("flags a build offered as a variant by two parents", func() {
311+
entries := variantFixture(
312+
entryWithVariants("base-a", "u://a", gallery.Variant{Model: "shared"}),
313+
entryWithVariants("base-b", "u://b", gallery.Variant{Model: "shared"}),
314+
plainEntry("shared", "u://shared"),
315+
)
316+
317+
violations := checkSingleVariantClaim(entries)
318+
Expect(violations).To(HaveLen(1))
319+
Expect(violations[0].Entry).To(Equal("base-b"))
320+
Expect(violations[0].Variant).To(Equal("shared"))
321+
Expect(violations[0].Detail).To(ContainSubstring(`already offered as a variant by "base-a"`))
322+
})
323+
324+
It("accepts two parents offering different builds", func() {
325+
entries := variantFixture(
326+
entryWithVariants("base-a", "u://a", gallery.Variant{Model: "one"}),
327+
entryWithVariants("base-b", "u://b", gallery.Variant{Model: "two"}),
328+
plainEntry("one", "u://one"),
329+
plainEntry("two", "u://two"),
330+
)
331+
332+
Expect(checkSingleVariantClaim(entries)).To(BeEmpty())
333+
})
334+
335+
// VariantParents takes the first claim per parent, so a repeat inside
336+
// one stanza hides nothing extra and is not this rule's business.
337+
It("does not treat one parent listing a build twice as two claims", func() {
338+
entries := variantFixture(
339+
entryWithVariants("base", "u://base",
340+
gallery.Variant{Model: "dup"},
341+
gallery.Variant{Model: "dup"},
342+
),
343+
plainEntry("dup", "u://dup"),
344+
)
345+
346+
Expect(checkSingleVariantClaim(entries)).To(BeEmpty())
347+
})
348+
349+
// VariantParents skips these too, so flagging them here would report
350+
// the same authoring mistake twice under two different names.
351+
It("leaves dangling and nested references to checkVariantReferences", func() {
352+
entries := variantFixture(
353+
entryWithVariants("base-a", "u://a",
354+
gallery.Variant{Model: "ghost"},
355+
gallery.Variant{Model: "nested"},
356+
),
357+
entryWithVariants("base-b", "u://b",
358+
gallery.Variant{Model: "ghost"},
359+
gallery.Variant{Model: "nested"},
360+
),
361+
entryWithVariants("nested", "u://nested", gallery.Variant{Model: "leaf"}),
362+
plainEntry("leaf", "u://leaf"),
363+
)
364+
365+
Expect(checkSingleVariantClaim(entries)).To(BeEmpty())
366+
})
184367
})
185368

186369
Describe("checkEntriesInstallSomething", func() {
@@ -322,6 +505,16 @@ var _ = Describe("gallery/index.yaml variant invariants", Ordered, func() {
322505
v := checkEntriesInstallSomething(entries)
323506
Expect(v).To(BeEmpty(), formatViolations(v))
324507
})
508+
509+
It("declares every entry name exactly once", func() {
510+
v := checkNoDuplicateEntryNames(entries)
511+
Expect(v).To(BeEmpty(), formatViolations(v))
512+
})
513+
514+
It("gives every variant build a single parent", func() {
515+
v := checkSingleVariantClaim(entries)
516+
Expect(v).To(BeEmpty(), formatViolations(v))
517+
})
325518
})
326519

327520
// The lint rules above check the catalog as text. This drives the real

0 commit comments

Comments
 (0)