Repository navigation
kb init: harvest a calibration set from the owner's review - #254
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c9c7100beb
ℹ️ 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 record is not None and record.verdicts and \ | ||
| any("section" not in v for v in record.verdicts.values()): | ||
| problems.append(f"the {stage} record predates the harvest (its verdicts carry no rule text); " | ||
| "re-run that stage's dry run to regenerate it") |
There was a problem hiding this comment.
Regenerate legacy records instead of returning cached dry runs
When all predecessor stages are dry runs created before this change and their repository inputs have not changed, this recovery instruction cannot work: rerunning the stage hits _Stage.run's unchanged inputs_digest cache and returns the legacy record without adding section or evidence, so every harvest remains blocked. Include a record-schema version in the digest or explicitly invalidate these legacy records before advising the rerun.
Useful? React with 👍 / 👎.
| others = [g for g in good if g[0].page != item.page or g[0].rule_id != item.rule_id] | ||
| others = [g for g in others if g[2] and g[2] != evidence and g[0].page != item.page] or \ | ||
| [g for g in others if g[2] and g[2] != evidence] |
There was a problem hiding this comment.
Require sibling evidence to come from another module
When no different-page donor exists, this fallback accepts evidence from another rule on the same page while labeling it as an “another module” mutation. If two same-page rules cite different or overlapping ranges that both support the target invariant, this creates an expected-reject case that a correct judge should pass; because synthetic cases count toward the trusted-bad floor, the resulting calibration set can incorrectly block activation.
Useful? React with 👍 / 👎.
| current = match.group("value").strip().strip("'\"") | ||
| if current == value: | ||
| return text | ||
| if current: | ||
| raise InitError(f"the manifest already names calibration set {current!r}; init only writes {value!r}") |
There was a problem hiding this comment.
Treat YAML null as an unset calibration set
For a valid manifest containing calibration_set: null or calibration_set: ~, parse_lifecycle treats the value as unset, but this textual check interprets null/~ as the name of another set and blocks harvesting. Normalize YAML null scalars as empty before deciding that the manifest already names a different calibration set.
Useful? React with 👍 / 👎.
modules (#251), deepen (#252) and harvest-calibration (#254) landed, so the skill no longer treats them as unimplemented: it runs the next stage and says what each one opens. The rules are unchanged: only MERGED PRs count as done when publishing, and dry-run records chain in dry-run mode. It notes that harvest refuses fewer than five owner-or-synthetic bad cases, and that auto_merge still needs kb calibrate and the shadow period (the skill never flips it). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Stage 1 of kb init (design kb-init v3): bootstrap a repository's knowledge base from its docs and seed pages, as one human-merged PR. - init_budget: hard per-stage ceiling by reservation. A generator call reserves threshold + one worst-case request (claude's --max-budget-usd only stops the next request); models without a price are refused before any call. Judge calls are accounted at a fixed cost. - init_support: InitRuntime (never KbRuntime, so never kb.db), the per-stage InitRecord, the upstream pin (bare mirror; the pin defaults to the mirror's HEAD, i.e. the upstream default branch, never an assumed main; safe export; PinnedObserver), model calls through the budget, claim/evidence and outside-knowledge path checks, the CI knowledge validators on the result, and InitPublisher (one deterministic commit on an exact base, pushed to a branch that must not exist, PR via the owner's gh; or a local dry run). Writes need ALLOW_PUSH=1, ALLOW_POST=1 and KB_INIT_GIT_AUTHOR (checked before any model call); private upstreams are always dry runs. INIT_PATHS allows knowledge/repos/_index.md, the shared list a new repository joins. - init_stages: run_stage + validate_change (the only check_changeset(bootstrap=True) caller). Skeleton: map call (entry page, architecture page, routes, general links), doc-invariant rules, adapted seed pages. Every rule is D5-filtered, evidenced and checked at the pin, then PLACED on a running tree before judging (a page that fills up sends the rule to a sibling page), so the advisory judge sees each rule where it is written; fail verdicts are stripped. Existing pages are only appended to; routes start from owners read deterministically from existing sub-directory entry pages (code paths they name that exist at the pin); findings on existing pages go to the PR checklist; a failed push is recorded as blocked. suggest_seeds ranks candidates without a model call. - InitConfig gains generator_call_usd and harness_overhead_bytes; build_doc_corpus takes optional globs (doc_files). - knowledge.init step, kb-init playbook, `kb init REPO --stage S [--dry-run] [--pin SHA]` and `kb init REPO --suggest-seeds`, dispatched before any ledger is opened. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
L1 reads any line starting with "## <Word>" as a rule ID (lifecycle.RULE_HEADING / ANY_RULE_HEADING), so a model-written "## Overview" or "## SERV-1 — x" in architecture.md or the entry page intro could collide with real rule IDs or masquerade as a rule. Every non-rule page the skeleton writes now goes through neutral_headings: an ATX heading outside code becomes a bold paragraph, any other line that starts with "#" is escaped, and such a line inside a fence is indented by one space. No line starts with "#", so the rule regexes (all anchored at column 0) can never match. The entry page's list heading is bold too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Stage 2 of kb init (design v3 §7.1-7.2): route every code module. - init_stages: the skeleton's flow becomes a _Stage base shared by every stage (pin, record, rule screening and placement, the deterministic checks as _conclude, the publication); _Skeleton keeps only its own steps. New: the stage chain. Each earlier stage must be merged (record published and gh pr view MERGED, so main holds it), or, for a dry run of this stage, dry-run: its snapshot is overlaid on main as the base and handed to the knowledge validators with this stage's change. A stage with nothing to change is recorded "empty", not blocked. Later stages keep the earlier stages' pin unless --pin says otherwise. - init_modules: scan_modules_at_depth at the pin (language from the adapter's repo.language; none or unknown -> no scan, checklist). A module an owner partly reaches is absorbed: its prefix is appended to that owner's scope_prefixes. Every other module gets one bounded generator call (file names, signatures, leading docstrings, never bodies) and a map card: a prose page (type architecture) in an existing components/<group>/ (index append-only) or a new group with its own _index.md and route owner, linked upward. Sections are bold labels: the knowledge format reads any "## <Word>" heading as a rule ID. The route table is only appended to (routes_append_only). Budget or card cap -> the cards written so far still make a PR; the rest is listed. - InitRecord.coverage: the stage's coverage report, rendered as a before/after table in the PR body. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Stage 3 of kb init (design v3 §7.3, §9 PR 3). - UpstreamPin.first_parent_changes: the PR window, read offline from the mirror: the files each of the last pr_window_count first-parent commits up to the pin changed, no older than pr_window_max_age_days before the PIN's commit time (a function of the pin, never of the clock). - init_deepen: modules by churn in the window, highest first. A module no route reaches, or whose owner already bears rules (the page itself, or a page in the same directory, has an active rule), is skipped. Every other module gets one generator call that reads its code at the pin (numbered, byte-bounded) and proposes rules with line-ranged evidence, which must lie inside the module. They go through the shared screening (D5, pinned evidence and claims, placement, advisory judge) and are appended to the owner's rule page, or rules.md beside it; existing rules are never edited. Stops at coverage_target (rule-bearing PR-weighted), when the budget runs out, or at the end of the list. The record reports routed and rule-bearing coverage before/after and the hot paths still without rules. - The shadow flip in the same PR: flip_to_shadow edits exactly the enabled / mode lines of the adapter's knowledge_lifecycle block (inserting enabled under the header if missing); comments and every other line are kept. _check_flip accepts only that manifest, check_flip_to_shadow, and parse_lifecycle on the head (on, shadow, same knowledge dir). A manifest without the block blocks before any model call. - init_modules.scan_modules is now a module function both stages use. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
b947786 to
1c6ceea
Compare
The last kb init stage (design v3 §11). After the deepen PR merged (a dry run chains on the dry-run snapshots), it compares what the advisory judge said about every rule init wrote with what the owner merged, and proposes adapters/<adapter>/kb-calibration/cases/*.json plus knowledge_lifecycle.calibration_set: kb-calibration as its own PR. - Stage records now keep the judged rule text and its pinned evidence in each verdict, so rules the judge stripped or the owner deleted can be turned into cases. - Labels follow the design table: kept pass/unsure/unjudged -> good (source owner); stripped fail -> bad (source judge); deleted or rewritten pass -> bad with override; deleted or rewritten unsure/unjudged -> bad (source owner). A rewritten rule's case holds the text the judge saw. - Cases are self-contained in the load_cases shape: the page without the rule as base, the judged text re-applied through apply_operations as head, evidence re-read from the pinned mirror. A case that does not pass L1 on its own is left out and noted. Case IDs carry the stage: an ID one stage's judge stripped can be reassigned by a later stage. - Deterministic mutations of good rules (broken path claim, shifted evidence range, negated invariant, a sibling module's evidence) balance the set and top owner-sourced bad cases up to five (a broken path must really be absent at the pin, a moved range must show different text); with fewer than five bad cases from the owner or from mutations the stage is blocked, since the judge's own fails cannot calibrate it; so it is with no good case at all (kb calibrate never passes without one). - The manifest edit is one line (set_calibration_set), checked with check_lifecycle_flip(allowed=CALIBRATION_KEYS) and parse_lifecycle; existing cases are never overwritten. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
c9c7100 to
a260acd
Compare
modules (#251), deepen (#252) and harvest-calibration (#254) landed, so the skill no longer treats them as unimplemented: it runs the next stage and says what each one opens. The rules are unchanged: only MERGED PRs count as done when publishing, and dry-run records chain in dry-run mode. It notes that harvest refuses fewer than five owner-or-synthetic bad cases, and that auto_merge still needs kb calibrate and the shadow period (the skill never flips it). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Summary
The last
kb initstage (design v3 §11):kb init <repo> --stage harvest-calibration. After the deepen PR merged (a dry run chains on the dry-run snapshots), it compares what the advisory judge said about every rule init wrote (skeleton, modules, deepen records) with what the owner merged, and proposesadapters/<adapter>/kb-calibration/cases/*.jsonplusknowledge_lifecycle.calibration_set: kb-calibrationas its own PR (kb/init-<repo>-harvest-calibration).Records keep the judged text. Each verdict now also stores the exact judged section and its pinned evidence, so rules the judge stripped or the owner deleted become cases. Older records are refused ("re-run that stage's dry run").
Labels (design table).
source: owner)source: judge)source: owner,override: true)source: owner)A rewritten rule's case holds the text the judge saw.
Self-contained cases in the
calibration.load_casesshape:baseis the page without the rule (plus its_index.md).headis the judged text re-applied throughops.apply_operations.upstream_text).init-<good|bad>-<stage>-<rule>), because an ID one stage's judge stripped can be reassigned later. The toy suite hits exactly this.Mutations (
synthetic: true) are deterministic, seeded by rule ID, with at most one per (rule, kind). They balance bad against good and top owner-sourced bad cases up to 5.broken_path: the path must really be absent at the pin.shifted_range: the moved range must show different text.negated: 强制↔禁止, MUST NOT↔MUST, …sibling_evidence: another module's evidence.Floors: the stage is blocked when fewer than 5 bad cases come from the owner or mutations (the judge can't calibrate itself), or when no good case remains (
kb calibratenever passes without one).Adapter edit: one line (
set_calibration_set). It is checked withcheck_lifecycle_flip(allowed=CALIBRATION_KEYS), plusparse_lifecycleon head namingkb-calibration. Existing case files are never overwritten, and a manifest that already names another set is refused.Deviations from the spec
InitRecord.verdicts, a small change ininit_stages._screen. Without it, stripped and deleted rules have no text to harvest.run_stage(..., "harvest-calibration")no longer raisesNotImplementedError; an unknown stage raisesInitError.Tests
test/test_kb_init_harvest.py(20 tests):load_casesreads the written setimc-check.shrc=0: full pytest, doc links and citations, SPEC freshness--strict, knowledge linters.20260930T123237.244014Z-195b4d7246-codeloop-i3-approve-mn0oiqec.md. Its earlier rounds forced stage-scoped case IDs, the absent-path and same-text mutation checks, and the no-good-case floor.Part 8/9 of kb init (design v3, sol-approved), stacked on #252.
🤖 Generated with Claude Code