Knowledge-as-CGS-substrate: grounding ledger, CGS metrics, insight sync, lint, Obsidian audit vault#35
Conversation
Brings InsightLabel.evidence_refs (gold evidence for CGS) and the /v1/insights collector endpoint contracts. Re-pin to the next tagged release once the agent-core PR merges. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
New grounding module implements the knowledge-as-CGS-substrate layer (arXiv 2605.06717): normalized OKF evidence refs, a deterministic recoverability verdict standing in for faithful() (label verdict wins, then the record's, then recoverability), per-concept usage rows (citations, loops, faithfulness/helpfulness/downstream rates), and the retrieval tie-break rank. insight_policy_metrics gains CGS (evidence F1 x faithfulness over surfaced+labeled decisions, None when no decision qualifies), mean precision/recall, per-loop breakdowns, and a stable learning_lift placeholder; the loader now also reads committed ledger/insights/ JSONL rows (created_at required for deterministic exports) and dedupes by id. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
write_exports derives exports/grounding.jsonl (+ manifest count + a grounding sqlite table with a tolerant store accessor) from committed insight decisions/labels only, keeping export --check reproducible. Quality gains a grounding report section plus never_cited (curated + top-level scope, only once production ledger records exist, honoring grounding_exempt), poorly_grounded (>=3 citations, faithfulness < 0.5), and verify_overdue (curated verify_by) warnings. Frontmatter schema documents verify_by/grounding_exempt. One fixture decision+gold label citing a real curated postmortem exercises the pipeline end to end (CGS = 1.0, one grounding row). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
grounding_rank (bounded citations x faithfulness) slots in as a late sort key after exact-match priority, authority tier, and total score, so outcomes change only on exact ties. Off by default via retrieval_grounding_tiebreak in knowledge.config.yml; flip only after an eval-diff review. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
hyrule-knowledge insights sync pulls /v1/insights from the agent-core
collector, validates contracts, runs the forbidden-payload scan, requires
created_at, dedupes against fixtures + existing ledger files, caps per
run, and appends to ledger/insights/{decisions,labels}/YYYY-MM-DD.jsonl —
records then ride the reviewed nightly PR (ledger/ stays outside any
auto-merge allowlist). insights metrics prints IDQ/CGS/silence per loop
for promotion decisions. The knowledge loop gains an opt-in phase-2
insight sync (insight_sync_enabled + collector URL + a per-day budget
tracked in the daily ledger).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
KNOWLEDGE_AUTO_MERGE=off|dry_run|on gates the workflow; bot-branch + bot-author guards, a regenerable-path allowlist (okf/generated, okf/observed, exports, reports — curated/ledger/src/schema/.github never auto-merge), and a quality non-regression check (critical==0, warnings <= base, concept count >= 80% of base) front a gh pr merge --auto so branch-protection required checks remain the actual merge gate. Start with a dry_run burn-in week. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adopts the LLM-Wiki vocabulary without any structural migration: hyrule-knowledge lint composes validation + quality findings (incl. the new grounding lint) + claim conflicts + stale claims into one read-only command; a minimal committed okf/.obsidian config lets operators open the bundle directly as an audit vault (relative links + graph view work natively, workspace state gitignored); scripts/auto-memory-to-ledger.py converts Claude auto-memory lessons into lesson_candidate learning events so workstation knowledge graduates into okf/curated/lessons via the existing reviewed promotion path. README documents ingest/query/lint and the grounding layer. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Same v0.39.0 workflow as the other loop repos on the unprivileged hyrule-public-pr runner, with repo-specific review instructions. Model env-pins stay in the workflow because .pr_agent.toml is only read from the default branch. Org grants (public-pr runner group + OPENROUTER_API_KEY selection) were applied 2026-07-10. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨No code suggestions found for the PR. |
README now defines all three insight-stream metrics (Insight Decision Quality with silence in the denominator; Context Grounding Score as evidence-F1 x faithfulness with the verdict precedence and null-not-zero semantics; Learning Lift as the frozen-vs-adapted placeholder pending network-operations#412), the collector->sync->ledger->exports->metrics pipeline, the OKF evidence contract, the grounding ledger and its lint/ retrieval consumers, and the determinism contract. Layout/commands/ automation/loop/consumption sections pick up ledger/insights, lint, insights sync|metrics (collector :8770), the KNOWLEDGE_AUTO_MERGE workflow, the loop's --insight-sync phase, and the three new export tables. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
agent-core#11 squash-merged as 4329ec9 and was tagged v0.8.0; the temporary branch-rev pin (or the stale v0.7.0 tag) is replaced by the durable release tag. Full suite green against the new contract (incl. the action_space ingest validator). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
1 similar comment
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1600915d32
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if concept is None and kind == "okf_context_pack": | ||
| # Context packs are report artifacts, not concepts; count them as | ||
| # recoverable when the id is present at all (they are committed). | ||
| resolved += 1 |
There was a problem hiding this comment.
When an insight cites kind: okf_context_pack, this branch increments resolved solely because the target string is present, without checking reports/context-packs.jsonl or the SQLite context_packs table. A typo or nonexistent context-pack ref is therefore scored as faithful, inflating CGS/grounding for any surfaced insight that uses context-pack evidence; pass known context-pack IDs into recoverability or treat them unresolved unless verified.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7ff9865: recoverability_verdict takes context_pack_ids and resolves an okf_context_pack ref only when it names a committed pack; without an index the ref is unresolved, never faithful-by-assumption. Threaded from the exporter (committed context-packs.jsonl) and the metrics CLI (context_packs table). Test: test_context_pack_refs_require_a_known_pack.
| base = json.load(open("base-coverage.json")) | ||
| head = json.load(open("reports/coverage.json")) | ||
| failures = [] | ||
| if int(head.get("critical_count", 1)) != 0: | ||
| failures.append(f"head critical_count={head.get('critical_count')} (must be 0)") | ||
| if int(head.get("warning_count", 0)) > int(base.get("warning_count", 0)): |
There was a problem hiding this comment.
In .github/workflows/auto-merge.yml, the non-regression gate compares the PR's checked-in reports/coverage.json. The required quality --check only marks reports stale when critical_count or concept_count differ (quality.py:211), so a bot refresh with a stale or incorrect warning_count can bypass this warning-regression guard while changing only allowlisted generated/report paths. Recompute coverage in this job, or make quality --check validate warning_count, before queuing auto-merge.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7ff9865: quality --check now treats a differing warning_count as stale reports (alongside critical_count/concept_count), so the required check itself guarantees the coverage.json the auto-merge gate compares is current — a bot refresh with a stale warning_count fails the required checks before the gate matters.
| gh api "repos/${GITHUB_REPOSITORY}/pulls/${PR_NUMBER}/files" \ | ||
| --paginate --jq '.[].filename' > changed-files.txt |
There was a problem hiding this comment.
The PR files API reports renames with the new filename and a separate previous_filename, but this allowlist only checks filename. A bot refresh that renames okf/curated/foo.md or ledger/... into an allowed path such as reports/foo.md would pass this confinement check while deleting human-reviewed content, so include previous_filename or use git diff --name-status against the base SHA for rename-aware blocking.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7ff9865: the confinement check now emits .filename AND .previous_filename from the PR files API, so both sides of a rename must satisfy the allowlist — renaming okf/curated or ledger content into an allowed path aborts auto-merge.
| except Exception as exc: # pydantic ValidationError and friends | ||
| summary["rejected"].append(f"invalid record: {exc.__class__.__name__}") |
There was a problem hiding this comment.
If _append_row raises an OSError in production (for example a full disk, read-only checkout, or permission problem under ledger/insights), this broad handler classifies it as an invalid record and insights sync still exits successfully with a rejection summary. That makes the Knowledge Loop treat a storage failure like bad collector data instead of alerting/retrying, so restrict this catch to validation errors and allow file I/O failures to propagate.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7ff9865: validation errors and file I/O are now separate — pydantic failures reject the record, but _append_row runs outside the catch so an OSError (full disk, read-only checkout, permissions) propagates and fails the sync/loop cycle as a storage incident. Test: test_ledger_write_failure_propagates.
| return _denied_pack(task=task, role=role, risk_level=risk_level, token_budget=token_budget, max_chars=max_chars, task_id=task_id, store=store, decision=decision.as_json()) | ||
|
|
||
| retriever = KnowledgeRetriever(store) | ||
| retriever = KnowledgeRetriever(store, grounding_tiebreak=retrieval_grounding_tiebreak()) |
There was a problem hiding this comment.
When hyrule-knowledge --config <alt> context-pack (or evals that build context packs) is used for an eval-diff review of retrieval_grounding_tiebreak, this call reads the default knowledge.config.yml in the current directory instead of the config that opened the store. The supplied config's tie-break flag is therefore ignored for context-pack retrieval, so thread the active config/flag into this path rather than reloading the default.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7ff9865: build_context_pack and run_evals accept grounding_tiebreak and the CLI threads retrieval_grounding_tiebreak(Path(args.config)) through, so --config <alt> eval-diff reviews exercise the supplied config's flag; the default-config read remains only as the fallback for cwd-based callers (MCP server).
|
|
||
| Returns a summary dict (imported/skipped/rejected counts + reasons). | ||
| """ | ||
| existing_decisions, existing_labels = load_insight_policy_fixtures() |
There was a problem hiding this comment.
When callers pass non-default decisions_dir/labels_dir (as the tests and any staging sync do), the duplicate check still loads only the default fixture/ledger paths, so records already present in the supplied directories are invisible and a rerun appends duplicates. Build the seen sets from the directories being written, or pass those paths into the fixture loader.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7ff9865: the dedupe seen-sets now union ids read from the actual target decisions_dir/labels_dir with the fixture/default-ledger loader, so reruns against custom directories skip existing records instead of appending duplicates. Test: test_sync_dedupes_against_custom_target_dirs.
Context-pack evidence refs are now verified against the committed pack ids (threaded from exports/store) instead of counting as faithful by mere presence — a typo'd pack ref no longer inflates CGS/grounding. insights sync propagates ledger write failures (full disk, read-only checkout) as storage incidents instead of misclassifying them as bad collector data, and dedupes against the actual target directories so reruns over custom dirs cannot append duplicates. build_context_pack / run_evals accept the grounding tie-break flag from the active config (the CLI passes --config's value) rather than silently reading the default knowledge.config.yml. quality --check now also treats a stale warning_count as stale reports, closing the auto-merge non-regression bypass, and the auto-merge path allowlist checks BOTH sides of renames (previous_filename) so curated/ledger content cannot be renamed into an allowed path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7ff9865a5d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| conflicts = find_conflicts(store) | ||
| stale = find_stale(store) |
There was a problem hiding this comment.
When hyrule-knowledge lint is run on a corpus with conflicting or expired claims, these calls pass neither a scope nor an explicit claim list. find_conflicts/find_stale default to an empty source set when scope is None, so the new lint command always reports empty conflicts and stale_claims even though the docstring advertises repo-wide claim checks; load the claim set from the store and pass it into both helpers.
Useful? React with 👍 / 👎.
| """ | ||
| by_id = {str(row["insight_id"]): row for row in decisions} | ||
| label_rows = [row for row in labels if str(row.get("insight_id")) in by_id] | ||
| labels_by_insight = {str(row["insight_id"]): row for row in label_rows} |
There was a problem hiding this comment.
When an insight is reviewed more than once, such as an operator correction, load_insight_policy_fixtures returns labels sorted by label_id, so this comprehension keeps whichever label sorts last rather than the latest created_at. CGS/evidence precision can therefore be computed with a stale verdict or gold evidence for production insight streams; choose the latest label per insight_id or reject duplicate labels before scoring.
Useful? React with 👍 / 👎.
| "insights", | ||
| "sync", | ||
| "--collector-url", | ||
| config.insight_collector_url, | ||
| "--max", |
There was a problem hiding this comment.
In the Knowledge Loop path, the collector sync never supplies --since or another cursor even though the collector request is limited. Once the collector has a page of already-imported records before newer ones, sync_insights skips them as duplicates, imports 0, and later loop runs request the same page again, so fresh production decisions/labels stop entering ledger/insights; persist and pass the last synced timestamp or page through the collector.
Useful? React with 👍 / 👎.
| "--max", | ||
| str(remaining), | ||
| ) | ||
| result = _run_checked(argv, repo, runner, report) |
There was a problem hiding this comment.
When --insight-sync is enabled with HYRULE_KNOWLEDGE_COLLECTOR_TOKEN, this call still uses the generic _run_checked environment path; _scrub_env only blanks OPENROUTER_API_KEY, so the collector bearer token must remain in the parent environment and is inherited by later refresh/validation subprocesses such as pytest. Pass the token only to the sync subprocess or extend the scrubber so unrelated commands do not receive the collector credential.
Useful? React with 👍 / 👎.
1 participant
User description
Context
Core of the agentic-loops v-next program (arXiv 2605.06717): the OKF corpus becomes the evidence substrate loop insights cite, and knowledge quality becomes measurable through use — not just declared at write time. Everything stays deterministic (
export --checkgreen) and human-reviewed (ledger/is excluded from any auto-merge path).Changes
faithful()(label verdict wins → record verdict → recoverability); per-concept grounding rows (citation counts, distinct loops, action counts, first/last cited, faithfulness/label-helpfulness/downstream-success rates); retrieval tie-break rank.insight_policy_metrics: evidence F1 × faithfulness over surfaced+labeled decisions (None when nothing qualifies, per the paper), mean precision/recall, per-loop breakdowns, stablelearning_lift: nullplaceholder. Loader now also reads committedledger/insights/**.jsonl(created_at required) and dedupes by id.exports/grounding.jsonl+ manifest count + agroundingsqlite table (tolerant store accessor for older pinned consumers); derived only from committed inputs.never_cited(curated+top-level scope, only once production ledger records exist,grounding_exempthonored),poorly_grounded,verify_overdue(curatedverify_by:); newhyrule-knowledge lintcomposes validation+quality+conflicts+stale.retrieval_grounding_tiebreak: false) — applied after exact/authority/score, so outcomes change only on exact ties; flip after an eval-diff review.hyrule-knowledge insights syncpulls collector/v1/insights→ validates/sanitizes/dedupes/caps →ledger/insights/{decisions,labels}/YYYY-MM-DD.jsonl(rides the reviewed nightly PR);insights metrics [--loop]prints IDQ/CGS/silence for promotion decisions; opt-in budgeted knowledge-loop phase-2 (--insight-sync).KNOWLEDGE_AUTO_MERGErepo variable, default off): bot refresh PRs confined tookf/generated|okf/observed|exports|reportswith quality non-regression queue behind branch-protection required checks; dry_run burn-in mode. Runbook:network-operations/docs/runbooks/knowledge-auto-merge.md.okf/.obsidianso the bundle opens directly as an audit vault;scripts/auto-memory-to-ledger.pygraduates Claude auto-memory lessons into lesson_candidate events via the reviewed promotion path; README documents ingest/query/lint + the grounding layer.Verification
All gates green:
validate okf,quality --check(0 critical / 44 warnings, unchanged baseline),export --check,eval --check,ledger --check,scan-secrets;pytest124 passed (13 new grounding/CGS/loader tests, 4 sync tests); ruff + mypy --strict clean.insights metricsprints IDQ=1.0 / CGS=1.0 over the fixtures;lintruns clean.🤖 Generated with Claude Code
PR Type
Enhancement, Tests, Documentation
Description
Diagram Walkthrough
flowchart LR collector["agent-core collector"] -- "GET /v1/insights" --> insight_sync["insight_sync.py"] insight_sync -- "validate & dedupe" --> ledger["ledger/insights/"] ledger -- "load_insight_policy_fixtures" --> grounding["grounding.py"] grounding -- "build_grounding_rows" --> exporter["exporter.py"] exporter -- "grounding.jsonl + sqlite" --> exports["exports/"] exports -- "grounding_rank" --> retriever["KnowledgeRetriever"] retriever -- "tie-break" --> query["query/resolve"] cli["cli.py"] -- "cmd_lint" --> quality["quality.py"] quality -- "grounding lint" --> findings["findings"] collector["agent-core collector"] -- "insight sync" --> ledger["ledger/insights/"] ledger -- "load_insight_policy_fixtures" --> exporterFile Walkthrough
13 files
Add lint, insights, and loop insight sync commandsNew grounding ledger module with CGS metricsAdd CGS metrics and ledger JSONL loadingExport grounding rows to JSONL and SQLiteNew insight sync module for collectorAdd grounding lint findings to qualityIntegrate insight sync into loop runnerScript to convert auto-memory to ledgerAdd grounding tie-break to retrieverPass grounding tiebreak to retrieverPass grounding tiebreak to retrieverAdd grounding method to KnowledgeStorePass grounding tiebreak to retriever4 files
Tests for CGS metrics and ledger loadingTests for grounding ledger moduleTests for insight sync moduleAdd grounded NOC fixture decision and label2 files
Add retrieval_grounding_tiebreak config flagNew auto-merge workflow for bot PRs1 files
Update eval run IDs and add governor_routing_context20 files