Skip to content

Kernel: what each check conclusion means - #86

Merged
don-petry merged 2 commits into
mainfrom
kernel/check-conclusions
Oct 15, 2026
Merged

don-petry merged 2 commits into
mainfrom
kernel/check-conclusions

Conversation

@don-petry

@don-petry don-petry commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Closes the story "Kernel: what each check conclusion means" from the epic "Epic: nine gaps in the Kernel, found in review" (#29). Until now, any conclusion other than success or pending counted as check_failed. So a required Lens that came back neutral admitted a fix Run with nothing to fix. Earlier in the proof of concept, neutral counted as passing (L-07).

Based on main, rebased after "Lifecycle check: bounds in the data for every cycle with no person in it" (#85).

What changes

This follows the architect's working ruling on the issue (option 1), and the review's ruling on order and attention:

  • The facts carry GitHub's own conclusion, and the port maps nothing. It only lowercases what GraphQL returns (SUCCESS, IN_PROGRESS).

    • In schemas/facts.schema.json, checks[].conclusion used to be a list of four allowed values. It is now any code matching ^[a-z_]+$.
    • The new counter-example examples/invalid/facts.a-conclusion-in-words.json keeps sentences out.
  • What each conclusion means is data. config/defaults.json gains check_conclusions, and schemas/config.schema.json describes it:

    • success is passing.
    • failure and timed_out are failing.
    • Normal waits are pending: queued, in_progress, pending, waiting, requested, expected, and a required check that has not reported.
    • neutral, skipped, cancelled, stale, action_required, startup_failure and error are unreadable. So is any value the map does not name.
  • The configuration cannot make neutral pass. The schema requires "success": "passing" and allows every other key only failing, pending or unreadable. Both new counter-examples are rejected: examples/invalid/config.conclusion-mapped-to-nothing.json (neutral mapped to null) and examples/invalid/config.a-neutral-check-passes.json. As a second guard, the Kernel treats any value other than these four readings as unreadable.

  • kernel/derive.py. A new function, check_reading(), reads each required check through the map.

    • A failed check is read first. It is readable and decisive: whatever a check with no verdict would say, the content goes back to the author.
    • With no failure, an unreadable check keeps the item in verifying with reason check_unreadable, before any waiting check is considered.
    • The duplicate-name behaviour that Kernel: a Lens is read only from the App that reports it #60 will fix (the last entry wins) is unchanged.
  • kernel/decide.py. check_unreadable defers: no Run is admitted and no act is made. It is in NEEDS_YOU_REASONS, so the item shows as needing the Maintainer. Almost every check with no verdict needs a person: an approval, a change to the required set, or a re-run.

  • tests/test_decide.py gains six tests:

    • test_a_neutral_required_check_defers: neutral, skipped, cancelled, stale, action_required, startup_failure, error and an unknown value, on each required Lens. Each is tried unreviewed, reviewed, and reviewed with a standing approval. Each gives defer, reason check_unreadable, attention needs_you, with no admission and no act. The other side: with every check green, the review is admitted, then the approval issued, then the item enqueued.
    • test_a_timed_out_required_check_goes_back_to_the_author
    • test_a_queued_running_or_absent_required_check_is_waited_for, including waiting, requested and expected
    • test_a_failed_check_is_read_before_an_unreadable_or_waiting_one, which pins the order
    • test_a_failure_with_cancelled_or_skipped_siblings_goes_back_to_the_author: check_failed and admit, on each Lens
    • test_a_reading_the_kernel_does_not_know_is_unreadable, the fail-closed reading of a map the schema would refuse

    test_a_neutral_check_does_not_pass_a_dependency_update, from "Kernel: a dependency update needs no model review" (Kernel: a dependency update needs no model review #70), still passes.

  • tests/test_tools.py gains test_only_success_can_pass_a_check. It shows the schema refuses every conclusion other than success mapped to passing, an unknown name mapped to passing, and any map where success does not pass.

  • sim/world.py.

    • A new fault, a_check_can_time_out_or_give_no_verdict, is listed in sim/grounding.py.
    • New invariants: no review-change is admitted and no act is made while a required check has no verdict; an author's fix Run may still be admitted for a failure or a conflict. A deferral on check_unreadable shows as needs_you.
    • settle() still has a person run again every check that did not pass. tests/test_simulator.py accepts an item resting on check_unreadable only if its attention is needs_you.
    • The invariants hold on 300 seeds.
  • tools/mutants.py gains eleven mutants and replaces "a neutral deterministic check passes", whose line is gone. 63 of 63 are killed.

    • "a neutral check is read as passing"
    • "a neutral check is read as failing"
    • "a conclusion the map does not name is read as passing"
    • "a timed-out check is read as no verdict"
    • "a running check is read as passing"
    • "a reading the Kernel does not know is trusted"
    • "an absent required check passes"
    • "a check with no verdict is ignored"
    • "a check with no verdict is read before a failed one"
    • "a check with no verdict does not defer"
    • "an unreadable check is not the Maintainer's to see"
  • Documents:

    • Design §3.3: the checks row, and a paragraph on the map, the order, the port's lowercasing and the Maintainer's attention.
    • Design §4.2 item 1, §0 and the Lifecycle diagram: "passed", not "reported".
    • Design §4.5: the new fault.
    • Design §4.6: a new boundary row.
    • L-07: its rule and checks.
    • Decision 0012: "passed" in its summary, and two boundary rows.

Acceptance criteria (from "Kernel: what each check conclusion means")

  • The facts carry GitHub's own conclusion, and the port maps nothing. check_conclusions is in the defaults and the schema. A value missing from the map is unreadable. Counter-example: examples/invalid/config.conclusion-mapped-to-nothing.json. The schema also refuses any map that reads anything but success as passing.
  • Tests written first: test_a_neutral_required_check_defers and test_a_timed_out_required_check_goes_back_to_the_author.
  • Three mutants: neutral read as passing, neutral read as failing, and an unmapped value read as passing.
  • Documents: design §3.3 checks and §4.2 item 1; L-07.

Left open, on purpose

  • The order is: failed, then unreadable, then waiting. A failure is readable and decisive, so I-13 holds: whatever a check with no verdict would say, the content goes back to the author. The other order would make an ordinary failure wait for a person. GitHub's default fail-fast reports one failure beside cancelled siblings, and a job that needs: the failed one is skipped. Both cases are boundary tests, and the reversed order is a mutant.
  • An unreadable check leaves the item in verifying and shows it as needing the Maintainer. The other unreadable reasons (mergeable_unreadable, facts_incomplete) still give no State.
  • The duplicate check name, where the last entry wins, is Kernel: a Lens is read only from the App that reports it #60's to fix. This change keeps that behaviour as it was.
  • Lowercasing GraphQL's conclusion is the port's job, and waits on adapters/.
  • a_check_can_time_out_or_give_no_verdict names no trial in section 6, so it is listed as unverified.

Checks and reviews

  • make verify and python3 -m tools.mutants pass on a git archive copy: 245 tests, OK (3 expected failures); 63 of 63 mutants killed.
  • The tests were written first.
    • These failed until the code changed: the neutral, waiting, order, failed-beside-cancelled-or-skipped and unknown-reading cases, the needs-you attention, the schema test, and the simulator's needs-you invariant.
    • test_a_timed_out_required_check_goes_back_to_the_author passed throughout, because every conclusion except success already failed. Its mutant shows the test now holds that.
  • A first review was clean on security, and its replay of the old test facts showed only the intended change. Its two majors are fixed in the second commit: the order of failed and unreadable checks, and an unreadable check now needs a person. So are its minors: the normal waits mapped to pending, the port note, and "reported" becoming "passed".
  • The second model family's read is the Gemini code review and the other bot reviewers on this pull request, as the Maintainer directed. It merges once it is clean and green at the reviewed head, under the Maintainer's standing approval.

Closes #35

🤖 Generated with Claude Code

View guided diff Turn on auto-fix

don-petry and others added 2 commits October 15, 2026 10:41
The facts carry GitHub's own conclusion for a required check, and the new
configuration map `check_conclusions` says what each means: `success` passing;
`failure` and `timed_out` failing; queued, in progress, pending and absent
pending; `neutral`, `skipped`, `cancelled`, `stale`, `action_required`,
`startup_failure`, `error` and any word the map does not name unreadable.

An unreadable required check defers, reason `check_unreadable`, in
`verifying`, and is read before a failed or a waiting one. No Run is admitted
and no act made. The configuration's schema refuses a map that reads anything
but `success` as passing.

Closes #35

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Following the review's ruling on #35:

- A failed required check is read before an unreadable one, then a waiting
  one. A failure is readable and decisive, so I-13 holds, and GitHub's
  fail-fast (a failure beside cancelled siblings) or a skipped dependent job
  no longer holds an ordinary failure.
- `check_unreadable` is in NEEDS_YOU_REASONS: almost every check with no
  verdict needs an approval, a change to the required set, or a re-run.
- `waiting`, `requested` and `expected` are normal waits, mapped to pending.
- The simulator's invariant is narrowed to no review and no act while a check
  has no verdict, and a deferral on one must show as needing a person.
- Design §3.3 says the port lowercases GraphQL's conclusion and does nothing
  else; "reported" becomes "passed" in §0, the Lifecycle diagram and
  decision 0012.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@don-petry
don-petry requested a review from a team as a code owner October 15, 2026 15:47
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ⚠️ Failed 2026-10-15T15:47:53.599930Z 19fbc9f PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@coderabbitai

coderabbitai Bot commented Oct 15, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 16 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Repository: combsmith/software-factory/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: a2f0173f-d555-47b6-a61f-0da475b3d01a
📥 Commits

Reviewing files that changed from the base of the PR and between 5a7cd6b and 19fbc9f.

📒 Files selected for processing: 20
  • config/defaults.json
  • docs/architecture/software-factory-design.md
  • docs/decisions/0012-lenses-report-findings-and-the-kernel-decides-convergence.md
  • docs/lessons.md
  • examples/invalid/config.a-change-by-a-person-skips-the-model-review.json
  • examples/invalid/config.a-neutral-check-passes.json
  • examples/invalid/config.conclusion-mapped-to-nothing.json
  • examples/invalid/config.no-deterministic-lens.json
  • examples/invalid/facts.a-conclusion-in-words.json
  • kernel/decide.py
  • kernel/derive.py
  • schemas/config.schema.json
  • schemas/facts.schema.json
  • sim/AGENTS.md
  • sim/grounding.py
  • sim/world.py
  • tests/test_decide.py
  • tests/test_simulator.py
  • tests/test_tools.py
  • tools/mutants.py
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@sonarqubecloud

Copy link
Copy Markdown

@don-petry
don-petry added this pull request to the merge queue Oct 15, 2026
Merged via the queue into main with commit ed2ee77 Oct 15, 2026
9 checks passed
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.

Kernel: what each check conclusion means

1 participant