Skip to content

fix(diagnose): require field on every topk_keys entry - #27

Open
MrBeldum wants to merge 1 commit into
llm-measurement:mainfrom
MrBeldum:fix/diagnose-topk-keys-require-field
Open

MrBeldum wants to merge 1 commit into
llm-measurement:mainfrom
MrBeldum:fix/diagnose-topk-keys-require-field

Conversation

@MrBeldum

@MrBeldum MrBeldum commented Oct 5, 2026

Copy link
Copy Markdown

Summary

  • Make the topk_keys field requirement explicit: a missing field flags the entry node so path/line are actionable (instead of a nil-node finding).
  • Omitting weight stays valid (connector default).
  • Table-driven coverage for missing/empty/unknown/valid fields; docs note in DIAGNOSE.md.

Test plan

  • go test ./internal/diagnose ./internal/cli
  • go vet ./...

Fixes #23

Missing field now flags the entry node so path/line are useful.
Omitted weight stays valid; cover missing/empty/unknown/valid cases (llm-measurement#23).
@kwisatzh

kwisatzh commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

thanks! will review this week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

diagnose: require a field in every topk_keys entry

2 participants