feat(cli): validate --as-of, audit's flags in OPTIONS, okf_audit schema test, site - #51
Merged
Conversation
… schema Four follow-ups from the okf audit review, none of which changed shipped behaviour beyond the new flag: - `okf validate` gains `--as-of`. BundleValidator.Validate already took a clock, but the verb exposed no way to set one, so its §5.5 "concept is stale" warning depended on the day it ran and could not be asserted in CI. `okf audit` had the flag; validate, which emits the warning, did not. Parsed before the positional, so an unvalued flag names itself. - The OPTIONS block now lists `--as-of` and audit's four filter flags. Audit's COMMANDS line is already 80 columns — the width of render's — so nothing more fits there. Real per-verb `--help` is a CLI-wide gap (`okf validate --help` fails identically today) and is recorded in ROADMAP.md rather than fixed for one verb. - okf_audit had no schema test, though eight sibling tools have one. The schema is what decides what a bare `okf_audit()` call means: all parameters optional, `stale` defaulting to true. Nothing pinned that. - README's spec-mapping row now names its sections like its neighbours. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The site restates what the repo already documents, and it was missing a verb: Home's command table, the CLI page's verb table, and the CLI docs page (synopsis row plus its own chapter) all stopped at seven verbs. The chapter's sample output is a real capture, like its neighbours — `okf audit bundles/acme_retail --as-of 2027-06-01`, pinned to a date where that bundle actually has stale concepts, since nothing in it is past its stale_after today. Scoped to that one fact on purpose; no broader site sync. Verified with npm run typecheck, npm run test (11 passing) and npm run build. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🟢 Approval recommended
The CLI behavior change is narrowly scoped, matches the stated intent, and is covered by focused new tests (including boundary and invalid-input cases).
Pull request overview
This PR follows up on the okf audit work by making staleness evaluation reproducible in okf validate, improving CLI discoverability of audit’s filter flags, adding a schema regression test for the okf_audit agent tool defaults, and updating the project website to document the audit verb.
Changes:
- Add
--as-of <YYYY-MM-DD>support tookf validate, wiring it through toBundleValidator.Validate(..., IOkfClock?)so §5.5 staleness warnings are CI-assertable. - Extend the global CLI usage text so audit’s filter flags are discoverable in the OPTIONS block.
- Add an
okf_auditJSON schema regression test (all-optional params;staledefaults totrue) and update the website docs/tables to include theauditverb.
File summaries
| File | Description |
|---|---|
| web/src/pages/Home.tsx | Adds okf audit to the home-page command table. |
| web/src/pages/docs/Cli.tsx | Documents the audit verb with captured example output and narrative guidance. |
| web/src/pages/Cli.tsx | Adds audit to the CLI page verb table and mentions key filter flags. |
| tests/OKF4net.Tests/CliTests.cs | Adds coverage for okf validate --as-of (boundary behavior, invalid date, and parsing before positional). |
| tests/OKF4net.Tests/Agents/AIFunctionExposureTests.cs | Adds schema regression coverage for okf_audit defaults/optionality. |
| src/OKF4net.Cli/OkfCli.cs | Implements validate --as-of by reusing the existing clock seam and updates Usage OPTIONS text for audit flags. |
| ROADMAP.md | Records the known CLI-wide gap for per-verb --help and why it’s not solved piecemeal. |
| README.md | Cosmetic wording alignment in the spec-mapping table row for §5.3–§5.5. |
| CHANGELOG.md | Notes the new okf validate --as-of flag under Unreleased “Changed”. |
Review details
- Files reviewed: 9/9 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Presence, value and positional were three independent scans of the raw argument array, and they disagreed with each other: - a token consumed as a valued flag's value was still seen as a flag by the presence check, so `okf audit b --type --stale` set the stale filter even though `--stale` was `--type`'s value; - only the positional scan honoured the documented `--` separator, so a `--json` after it still switched the output format, and a path named like a flag was read as one. The three helpers are replaced by CliArgs.Scan, which walks the tokens once and records what each one is. Valued flags are declared per verb, at the single point where the scan happens, so a call site can no longer forget them. Behaviour is unchanged for every well-formed invocation: 1046 tests, including every golden, passed before the two regression tests below were added. Both defects were found by review, and neither had coverage — no test in the suite used `--` at all. The two new tests pin them: the first was reproduced against the old code before the fix. Also from the same review: - The sample README claimed "which concepts have never been verified by a human?" isolates skills/run-on-bq. It does not: okf_audit defaults stale to true, so the question as posed asks "stale AND unverified" and correctly returns nothing, that concept having no stale_after. The README now says which arguments the question actually needs. - okf_audit's [Description] — what the model reads when deciding how to call it — still said "the concepts needing attention" even for a stale:false selection of fresh concepts, which the renderer already headed "selected". - Nothing invoked okf_audit through MCP's own argument conversion; the new MCP test calls it and asserts the filter took effect (verified by dropping the filter and watching it fail). - Two French typos in the design spec, and a plan snippet still expecting the pre-review heading. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two things shipped after the site was synced, and neither reached the documentation: - `--as-of` was documented only inside audit's own chapter, even though `okf validate` gained it in this same branch — and validate is where it matters most, since its §5.5 staleness warning is the one diagnostic that silently depends on the day CI runs. Now in the README (for both verbs) and in the site's validate chapter. - Both library reference pages list ConceptSearch, the shared scorer, but had no row for ConceptAudit, its sibling: the shared corpus-level query behind `okf audit` and `okf_audit`. If one earns a row, so does the other. Found by asking where else the docs live rather than assuming the site sync had covered everything; the site was updated before these two changes existed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…wins A /simplify pass over both PRs. Two of the four review angles independently pointed at the same redundancy in CliArgs, and chasing it surfaced a regression I had introduced in the previous commit. - `_flags` (a presence set) beside `_values` (a value map) tracked the same thing in two collections that every write site had to keep in sync. One `Dictionary<string, string?>` says it once: key absent means the flag was not given, a null value means present-but-unvalued. - `_afterSeparator` beside `_positionals` was a special case bolted onto a list whose tail nothing ever read — `Positional()` only ever wanted the first token. One `string?` field, assigned with `??=` for a normal positional and overwritten when `--` has a token after it, reproduces every path exactly. Note the guard: one reviewer proposed clearing the list at the separator instead, which would break `okf audit b --` (trailing separator, nothing after it) by discarding the positional that preceded it. - **Regression fixed:** rewriting the helpers into CliArgs silently changed a repeated flag from first-occurrence-wins to last-occurrence-wins, because the value map was overwritten on each occurrence. The design spec (§4.1) documents first-wins, inherited from the original `Array.IndexOf` lookup. Nothing tested it, so nothing caught it. Restored, with the later occurrence still consuming its own value so it cannot reach the positional. - `"--as-of"` was spelled as an independent literal in three places, none tied to the others; a typo would have broken the flag on one verb only. Now a const. Two tests added for the two behaviours that had no coverage: the trailing separator, and the repeated flag. Both were verified against the running CLI, not only through the suite. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ess holes - The site said "Seven commands" / "Seven subcommands" directly above tables that now list eight. The commit that added the row had to move the count with it; this is the public site. - AsOfFlag had inherited AuditValuedFlags' doc comment, leaving two <summary> tags on one member and the array undocumented. My own sed did that. - AuditReport.Findings' XML doc still promised "sorted by concept id (ordinal)" — the exact contract commit 0567704 changed the code away from, and the one a consumer would code against. - ConceptAudit.Run seeded its counters from a hand-written list of enum members, so a fourth TrustTier or ConceptStatus would have made the increments throw KeyNotFoundException — breaking the type's documented "never throws on data" contract precisely where AuditVocabulary exists to stop that drift. Seeded from the vocabulary instead. - CliArgs.Value() reported "requires a value" for a flag the scan was never told takes one, blaming the user for what is a caller bug. It now distinguishes the two. - Null_clock_falls_back_to_today_in_utc sampled UtcNow on both sides of the call and would flake on a run crossing midnight UTC — in the one file whose header claims every test pins the date. - CmdAudit's comment still explained the free-function Positional helper this branch deleted. Documented, not reverted: making the three scans one also made `--` apply to every verb rather than to the positional lookup alone, so `okf fmt -- file -w` no longer rewrites the file — `-w` after a separator is a filename, which is what `--` has always meant. That is the intended rule, but it changes shipped behaviour on four verbs, so it now has a CHANGELOG entry and a test on `fmt`, the verb whose flag has a side effect on disk. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The site said only that "a path beginning with - can be passed after a -- separator" — true before, and now half the story. Since the three argument scans became one, everything after the separator is an argument on every verb, so `okf fmt -- notes.md -w` treats -w as a filename. Documents that, the working spelling, the same rule for a value belonging to an option, and adds --as-of to the global options it lists. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Taken in the order the window closes, since `okf audit` is in no release yet and its JSON is still free to move. **3 — one spelling per document.** `--json` named trust tiers two ways in one payload: camelCase in the counts object, the vocabulary's hyphenated names in findings and in the replayed query, so `counts[finding.trust]` returned undefined. The counts object now uses unverified / machine-confirmed / human-reviewed, in ladder order, which also makes TrustTiersInOrder's "JSON serializes in this order" true of the object it claims to govern. The hand-authored golden is revised deliberately, for an intentional output change — the values are identical, only the key names and their order move. **4 — `okf validate --json` reports `asOf`.** The flag exists so a CI verdict is reproducible; an archived report that never says which date it was evaluated against cannot be told apart from an unpinned run. The date is now resolved once in CmdValidate, so the validator and the document cannot disagree about it. **1 — `okf_audit(stale)` follows the CLI's rule.** It defaulted to true, so asking "which concepts were never verified by a human?" silently also demanded staleness and answered "none" whenever the unverified concept had no stale_after — which is the bundle the sample ships. Now `bool?`: unset means the stale worklist when nothing else is filtered, and no staleness constraint once another filter is given, exactly as the CLI behaves. An explicit stale still wins. Verified over a real MCP session, not only in tests: the sample's advertised question now returns skills/run-on-bq. The sample README loses the workaround paragraph it needed. **2 — a blank `type` is no filter.** §11 forbids an empty frontmatter type, so filtering for one could only ever select nothing, while trust and status returned a usage message for the same input. Normalized once in ConceptAudit rather than at each boundary, so every caller inherits it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
# Conflicts: # src/OKF4net.Agents/OkfBundleTools.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-ups from the
okf auditreview. Independent of #50 — both branch fromdev.okf validate --as-of <YYYY-MM-DD>BundleValidator.Validatealready accepted anIOkfClock, but the verb exposed no way to set one — so its §5.5concept is stalewarning depended on the day it ran and could not be asserted in CI.okf auditshipped with--as-of; the verb that actually emits the warning did not. Default behaviour is unchanged.Parsed before the positional is resolved, so
okf validate --as-ofreports--as-of requires a valuerather than hiding it behindmissing <bundle>.Audit's flags in OPTIONS
Audit's COMMANDS line is already 80 columns — exactly the width of
render's — so its six optional flags cannot be hinted there. They now appear in the OPTIONS block, the way--out <dir>does.What this deliberately does not fix: there is no per-verb
--help.okf audit --helpprintserror: missing <bundle>— and so dookf validate --helpand every other verb, because the CLI has one global usage block and resolves the positional before looking at--help. That gap is CLI-wide; fixing it forauditalone would create the inconsistency it looks like it solves. Recorded inROADMAP.mdto be done for all eight verbs at once, including the exit-code change it implies.okf_auditschema testEight sibling tools have an
okf_X_schema_*test;okf_audithad none. The schema is what decides what a bareokf_audit()call means: every parameter optional,staledefaulting totrue. A schema regression that marked a parameter required, or dropped the default, would silently change the tool's meaning with no C# signature changing.Site
web/was missing a verb in three places: Home's command table, the CLI page's verb table, and the CLI docs page (synopsis row plus its own chapter). The chapter's sample output is a real capture like its neighbours, pinned with--as-of 2027-06-01because nothing in that bundle is stale today.Scoped to that one fact — no broader site sync.
web/only deploys on push tomain, so this rides along with the nextdev→main.Verification
TreatWarningsAsErrors;dotnet format --verify-no-changesclean.npm run typecheck,npm run test(11 passing),npm run buildall clean.validate's default output is byte-identical.Also in here, one cosmetic line: README's spec-mapping row now names its sections like its neighbours.
🤖 Generated with Claude Code
https://claude.ai/code/session_01TDM9Aozt9YciNJ3oncrFxG