ci: enforce gofmt, and format the files that were failing it

[bawoodruff]

Four files have been sitting unformatted on main, unnoticed because nothing checked. CI built, vetted, and tested — but never looked at formatting.

• src/kg/add_observation.go
• src/kg/helpers.go
• src/kg/perf.go
• src/markitdown/converter/openai.go

Both halves land together on purpose. Adding the gate alone turns main red; formatting alone drifts back the next time someone skips gofmt.

The formatting change

Whitespace only — struct tag alignment and one blank line, no semantic difference. All three modules build and test green on CI's go1.26.2.

None of the open PRs (#4, #5, #7, #11) touch these four files, so their merge refs pick up the formatted versions without conflict.

The gate

Runs first in each module leg — cheapest signal, and the most irritating one to discover after a full test run.

- name: Format
  run: |
    unformatted=$(gofmt -l .)
    if [ -n "$unformatted" ]; then
      echo "::error title=gofmt::run 'gofmt -w' on these files:"
      echo "$unformatted"
      exit 1
    fi

gofmt -l prints offending files and exits 0 either way, so the assertion is that its output is empty, not its exit code. Verified by dropping a deliberately misformatted file in and confirming the check fails on it — a naive gofmt -l . && ... would have silently passed.

🤖 Generated with Claude Code

[github-actions]

VERDICT: approve

Review Summary

• Files reviewed: .github/workflows/ci.yml, src/kg/add_observation.go, src/kg/helpers.go, src/kg/perf.go, src/markitdown/converter/openai.go
• Overall verdict: APPROVE

Critical Issues (must fix before merge)

None.

Major Issues (should fix)

None.

Minor Issues (optional)

• .github/workflows/ci.yml:59-66 — The new Format step relies on set -e (GH Actions' default bash -eo pipefail) to fail the job if gofmt -l . itself errors (e.g. a file with a parse error), since a failing command substitution in an assignment (unformatted=$(gofmt -l .)) still trips -e. This does fail safe today, but the failure would surface as a generic "Process completed with exit code 1" rather than the custom ::error title=gofmt:: annotation. Not a defect, just a readability nit if you ever want a more explicit message for that path (e.g. gofmt -l . > /tmp/out || { echo "::error::gofmt failed to parse one or more files"; exit 1; }).

Security Findings

None.

Positive Observations

• The gofmt -l exit-code pitfall (it exits 0 regardless of findings) is correctly avoided by asserting on output emptiness rather than exit status — and the PR description explicitly documents that this was verified with a deliberately misformatted file.
• The new Format step correctly inherits the job's defaults.run.working-directory: src/${{ matrix.module }}, so gofmt -l . is properly scoped per module leg rather than accidentally linting the whole monorepo (which would matter given the three independent go.mods with different Go versions).
• All four reformatted files are genuinely whitespace-only changes (struct tag column alignment, one blank line added/removed) — no semantic drift, confirmed by reading the full diff context.
• Bundling the gate and the fix in one PR (rather than adding the gate first) avoids an immediate red main, and the PR body correctly reasons about which open PRs could conflict.
• Self-validating: CI already ran and passed (kglib, kg, markitdown all SUCCESS) with the new gofmt gate active, confirming no other unformatted files lurk elsewhere in any of the three modules.