🐛 fix(template): accept a v0.1 flat-string sources list for required_fields

[cwest]

Why

--templates with required_fields: [sources] no longer accepted a v0.1
flat-string sources: list, so well-sourced v0.1-shaped nodes were reported as
missing sources. Regression introduced by #61 (8857f65 → 78ae7c3).

§13.1 promises v0.1 bundles stay consumable by a v0.2 consumer under documented
fallbacks; this was a case where that promise did not hold. It also inverted the
curation incentive: a node with zero sources and a node with five well-labelled
ones produced the same finding, so the authors who did the sourcing work got the
warnings.

Root cause

hasNonEmptyField's sources case answered a structured-parse question
(Node.SourceCitations() > 0) for what is a presence question.
Node.Sources() drops any entry that is not a structured mapping (§5.1 requires a
resource per entry), so a list of plain strings parsed to zero and fell through
to the body # Citations fallback — which a frontmatter-sources v0.1 node does
not have. The flat-string form fell between the two recognised paths.

What

• Scope the loosening to the template presence check only: a new
  sourcesFieldPresent helper counts a non-empty frontmatter sources list of
  any entry shape (v0.2 structured OR v0.1 flat-string, non-blank) as
  satisfying the required field, retaining the §13.1 legacy body # Citations
  fallback (§5.1, §13.1).
• Node.Sources() / Node.SourceCitations() stay strict and unchanged —
  analyze, coverage and freshness keep the structured-provenance meaning.

Verification — run output, not claims

Positive controls (the fix fires on the bad case)

• v0.1 flat-string sources: (url|label) → no drift (unit + binary).
• Bare-URL flat-string sources: → no drift (unit).
• v0.1 fixture bundle at binary level, validate --templates:

# BEFORE (main, 78ae7c3)
warning knowledge/thing/overview.md: missing required field: sources (template knowledge)
1 drift warning(s)
# AFTER (this branch)
OK: bundle conforms to the OKF spec floor and team templates

→ 1 → 0.

Negative controls (load-bearing — the fix stays silent on the good case)

• sources: absent → still warns missing sources (existing test).
• sources: [] empty list → still warns (TestTemplateDrift_EmptySourcesList_Drifts_Section5_1).
• sources: all-blank/whitespace entries → still warns (TestTemplateDrift_BlankSourcesEntries_Drifts_Section5_1).
• v0.2 structured sources: [- resource: …] → still passes (existing test).
• No frontmatter sources + legacy body # Citations → still passes (existing test, §13.1 intact).
• Criterion 9 invariant: Node.Sources() unchanged — a flat-string list
  still parses to 0 structured entries
  (TestNodeSources_FlatStringList_ParsesToZero_Section5_1), proving the
  loosening did not leak into provenance analysis.

Real corpus — ~/src/knowledge-base/bundles/knowledge (AGENTS.md layer 3)

Our corpus is fully v0.2-shaped (0 flat-string sources nodes), so it is the
negative control: the fix must move nothing.

Command	Before	After
validate … --templates	OK (0 warnings)	OK (0 warnings) — 0 → 0
lint --strict …	1 finding	1 finding (byte-identical)
analyze …	—	byte-identical (diff empty) — proves criterion 9 held in practice

Gates

gofmt -l .          # empty
go vet ./...        # clean
go test ./internal/okf/ ./cmd/ -run Conformance -race    # ok
go test ./... -race                                      # ok (all packages)

Closes #79

[cwest]

The change itself is right and I'd take it as-is: the sources case in hasNonEmptyField was asking a structured-parse question where a presence question belongs, and scoping the loosening to a new sourcesFieldPresent helper while leaving Node.Sources() strict is exactly the correct shape. The test matrix covers both directions — flat-string and bare-URL satisfy the field, while empty list, all-blank entries, and an absent key still drift — and the criterion-9 invariant test proves the loosening does not leak into structured provenance. I verified end to end against the real corpus (validate --templates 0 to 0, lint --strict 1 to 1, analyze byte-identical) and reproduced the fix through the CLI on a v0.1 flat-string bundle: the missing-sources warning fires on the base binary and is gone after the fix, and returns when sources is removed. §5.1/§13.1 are cited in the code, tests, and PR body.

What blocks this from being merge-ready is not in this diff. CI (build-test) is red and the merge state is UNSTABLE, but every failure is in internal/apiserver (the lexical_gate search tests), a package this PR does not touch. That breakage is already present on main on its own — the push build for the current main tip is red with the identical six failures, and they reproduce on a clean checkout of main with no PR involved. The merge ref inherits them, so this PR's CI can't go green until main is fixed and this branch is rebased onto a green tip. The apiserver allowlist rejects lexical_gate before the handler that knows how to read it ever runs.

Holding this here: the branch needs to sit behind a fix to the apiserver search-param handling on main, then rebase onto a green main so build-test passes. Nothing to change in the template code.

[cwest]

Verified this independently, control versus candidate, both built from a fresh clone with Go 1.26.5. Control is 69c3673 (current main); candidate is 31eab7f (this branch head).

The fix holds, and the negative controls hold with it

Synthetic fixture, seventeen nodes under one template declaring required_fields: [title, sources], each node isolating a single entry shape. Only the shape differs between them.

node shape	control 69c3673	candidate 31eab7f
v0.1 flat-string list	warns	passes
flat-string list with one blank entry and one real one	warns	passes
v0.2 structured list	passes	passes
mixed flat-string and structured in one list	passes	passes
legacy body # Citations, bracket-key entries	passes	passes
legacy body ## Citations, ordered entries	passes	passes
no sources key at all	warns	warns
sources: []	warns	warns
list of only blank strings	warns	warns
list of numbers	warns	warns
bare scalar string	warns	warns
single mapping rather than a list	warns	warns
# Citations holding only a status-key legend	warns	warns

Two rows move and eleven stay put. That is the shape I was hoping for: the relaxation reaches the flat-string list and stops there. A list of numbers and a list of blank strings still warn, so this is not the blunt "any non-empty list satisfies it" reading, which would have traded a false positive for a false negative.

I confirmed the boundary the doc comment claims. On the candidate, Node.Sources() still returns zero for a flat-string list, so the strict provenance accessor is untouched and only the presence question moved. The two surfaces now deliberately disagree, which is the right call, and the comment on sourcesFieldPresent says so plainly enough that the next reader will not mistake it for an oversight.

One correction to my own reproduction in #79. I first wrote the body-citations control with plain - https://... bullets and saw it warn on both binaries, and briefly took that for a second gap. It is not. citationCount counts bracket-key and ordered entries, and a plain bullet was never an entry. My fixture was wrong, not the code. Rewritten in the valid form, the body-citations path passes on control and candidate alike.

Real corpus

Same private v0.1-shaped corpus that surfaced #79, run read-only through validate --templates on both binaries.

measure	control 69c3673	candidate 31eab7f
nodes flagged missing required field: sources	49	2
other missing required field warnings	17	17
total drift warnings	77	30

Diffing the two full outputs line by line: forty-seven lines removed, zero added. A pure subtraction, no finding relocated and no new finding anywhere in the corpus. The seventeen other required-field warnings not moving is the control that says this stayed inside the sources case.

I checked the two nodes still flagged. Both genuinely carry no sources key and no citations heading. True positives, correctly kept.

The red check is inherited, not yours

Worth stating clearly, because the red X on this branch is misleading. This branch is cut from 78ae7c3, before #80 and #81 landed, so it does not contain them. Built and tested alone, it is green, internal/apiserver included.

What CI tests for a pull request is the merge of this head into main, and main is currently red. I built that merge locally and reproduced exactly the six TestSearch_Gate* and TestSearch_CrossSurfaceParity failures that main carries on its own. None of them touch template code.

So the sequencing is what it looks like: land #83 and this clears. I tested that directly rather than assuming it. Merging this branch on top of #83 merges cleanly, the two touch disjoint files, go test ./... is green across every package, gofmt -l is empty and go vet ./... is clean. The combined binary's validate --templates output on the real corpus is byte-identical to this branch alone, which is the control proving #83 has no effect on template behavior in either direction.

[cwest]

The template change itself is correct, well-scoped, and fully tested — nothing to change in the code. I cherry-picked this commit onto the current tip of main and ran the whole gate green there: gofmt clean, go vet clean, the conformance suite passes, the full suite passes under -race, all the new presence/negative-control tests pass, and the real-corpus run is unchanged (validate --templates OK→OK, lint --strict byte-identical, analyze byte-identical). The §5.1/§13.1 citations are in the code, the test names, and the PR body.

The one thing standing between this and ready is stale base state. This branch is still built on an older main whose CI was red for an unrelated apiserver reason, and that red result is what build-test is reporting here — not anything in this diff. That upstream apiserver issue has since been fixed on main, so main is green again now. The branch just needs to be rebased onto the current tip of main and pushed; CI will re-run green and this becomes ready.

Concretely: rebase wt/t_fecc790d onto the latest origin/main (three commits ahead of this branch's base), force-push, and let CI re-run. No code edits needed. Once the rebased CI is green I'll re-review and it passes.

Leaving this in draft until the rebased CI is green.

[cwest]

No changes needed.

The template presence check now answers the right question. hasNonEmptyField's sources case delegates to sourcesFieldPresent, which treats a non-empty frontmatter sources list as satisfying the required field regardless of entry shape — the v0.1 flat-string form and the v0.2 structured form both pass, and the §13.1 body # Citations fallback is retained. Node.Sources() and SourceCitations() are left strict and unchanged, so the loosening is scoped to presence and does not leak into provenance analysis (§5.1, §13.1).

The test matrix carries the change: flat-string with labels and bare-URL forms satisfy; an absent field, an empty list, and all-blank entries still drift; the structured form and legacy body-citations still pass; and a flat-string list still parses to zero structured entries through Node.Sources() — both controls present in each direction.

Verified on the rebased head against current main:

• Conformance suite (-race): green.
• Full suite (go test ./... -race): all packages green, including the API server package that was red before the rebase.
• gofmt -l empty; go vet clean.
• Real corpus (~/src/knowledge-base/bundles/knowledge): validate --templates 0 → 0 (the corpus is v0.2, so a count that does not move is the control that proves the change hides nothing); lint --strict byte-identical; analyze byte-identical.
• CI build-test passing on the rebased base; branch merges cleanly.

Ready to go.