Skip to content

build: include available experiment suites in root checks - #90

Merged
AidenJohnston merged 1 commit into
mainfrom
t3/check-experiments
Sep 27, 2026
Merged

AidenJohnston merged 1 commit into
mainfrom
t3/check-experiments

Conversation

@SamGu-NRX

@SamGu-NRX SamGu-NRX commented Sep 27, 2026 •

Copy link
Copy Markdown
Owner

What changed

make check now includes scoring, Measure Lab, evals, recon, and meter-closeup when each suite's entry file exists. Each suite also has an explicit root target that fails with a specific message when the component has not landed. Existing server, web, and iOS targets are unchanged.

The meter-closeup target invokes only its check target. It does not run the private-key leak scan.

Why

These components arrive in separate PRs: #4, #7, #12, #20, and #16. The presence checks let this PR land before or after any of them, without five branches editing the same root Makefile. Once all five land, the guards should become unconditional dependencies so a deleted folder fails.

Verification

bash tests/makefile.sh passes locally with GNU Make 3.81. It uses stub commands to check all 32 presence combinations, exact target commands, explicit missing-suite errors, and propagation of suite failures. A new read-only Ubuntu workflow runs that test without installing component toolchains.

git diff --check passes. These tests check dispatch, not the component suites themselves. A full make check was not run on this sparse worktree; CI run 36294652145 passes the dispatch test on Ubuntu at 9d66b31.

Risk

The component entry points must remain compatible with these targets. Missing entry files are deliberately skipped by the umbrella target during the merge transition. An explicit suite target still fails.

Checklist

  • The scope is focused; related changes belong together
  • I explained what changed and why
  • N/A: No visible UI changes or motion

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Developer Tools
    • make check now includes scoring, Measure Lab, evals, recon, and meter close-up checks when those suites are present on the branch. Missing required files or failed checks stop the run with an error.
  • Documentation
    • Updated the README and contributor guidance to reflect the expanded checks.
  • Quality Checks
    • Added automated validation of Makefile check behavior, run on relevant pushes and pull requests.

Co-Authored-By: Claude Code <noreply@anthropic.com>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 27, 2026 •

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 ✅ Completed 2026-09-27T04:36:56.915484Z 9d66b31 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.

@coderabbitai

coderabbitai Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 3420b01e-6692-4f0b-b4d3-ac38f4a26805

📥 Commits

Reviewing files that changed from the base of the PR and between f852207 and 9d66b31.

📒 Files selected for processing (6)
  • .github/workflows/makefile.yml
  • AGENTS.md
  • CONTRIBUTING.md
  • Makefile
  • README.md
  • tests/makefile.sh

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The root Makefile adds five optional experiment suite targets to make check. It includes each target when its required file exists. A Bash test validates dispatch behavior, and a GitHub Actions workflow runs that test for relevant changes.

Changes

Make check suite dispatch

Layer / File(s) Summary
Configure core and optional suite dispatch
Makefile, AGENTS.md, CONTRIBUTING.md, README.md
make check now runs the core suites and each of five experiment suites when its required file exists. The suite targets run their configured commands or delegate to the corresponding suite Makefile. The documentation describes the added targets and expanded check scope.
Test and run the dispatch checks
tests/makefile.sh, .github/workflows/makefile.yml, CONTRIBUTING.md
The Bash test checks all 32 combinations of optional suites, target commands, missing-suite errors, and command failures. The workflow runs the test for relevant pushes and pull requests.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant MakeCheck as make check
  participant CoreSuites as server/web/ios
  participant OptionalTargets as optional suite targets
  participant SuiteCommands as suite commands
  MakeCheck->>CoreSuites: Run core suites
  MakeCheck->>OptionalTargets: Include targets whose required files exist
  OptionalTargets->>SuiteCommands: Run target-specific checks
Loading

Merge Risk: ⚪ Minimal · up to 9d66b

The optional suite dispatch is ready to merge after normal checks; no actionable issue is established by the supplied evidence.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 9d66b

The change does not show a new production attack path. Its main risk is that, after a suite has arrived, removing its entry file would silently remove it from the root check. The new workflow checks dispatch behavior with stubs, not the suites themselves.

Retained concerns

  • Low · architecture · inferred: Once a suite has landed, removal of its entry file would silently remove its validation from the umbrella check until the planned unconditional prerequisites replace the presence guards.
Security review details

Security Blast Radius

  • inferred — The inspected new CI execution path is limited to stubbed dispatch validation, rather than execution of the experiment suites or a production service.

Trust Boundaries and Controls

  • observed — The workflow grants read-only repository contents access and checks out code without persisting credentials.

Resilience and Maintainability Implications

  • inferred — Conditional suite inclusion limits failures during staggered merges but can allow validation coverage to drift silently after a suite has landed.

Hardening Proposals

  • proposed — At the documented end of the merge transition, make the five suites unconditional prerequisites so a missing entry file fails the umbrella check.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 1 files. (5 skipped: 5… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: adding available experiment suites to the root checks.
Description check ✅ Passed The description includes the required What changed, Why, Verification, Risk, and Checklist sections. It explains the behavior, rationale, testing results, limitations, and the absence of UI changes.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 1 files. (5 skipped: 5 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR

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.

@github-actions github-actions Bot added the size:M 30-99 effective changed lines. label Sep 27, 2026
@codemetrics-ai

Copy link
Copy Markdown

Note

Review skipped

Codemetrics is installed on this repository, but it hasn't been added to your team yet.

To enable AI code reviews for this repo, go to your dashboard and add house-scanning-master to your team's repositories.

@SamGu-NRX

Copy link
Copy Markdown
Owner Author

@greptileai

@greptile-apps

greptile-apps Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

[Medium risk] Build system adds conditional experiment suite targets.

The PR appears safe to merge.

Summary

The PR adds conditional experiment-suite dependencies to make check, explicit targets that fail when a suite is absent, a stub-based dispatch test, and an Ubuntu workflow for that test.

  • The dispatch test passed locally.
  • The available suite definitions match the new target names and commands.
Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A["make check"] --> B["server · web · ios"]
  A --> C{"Suite entry file exists?"}
  C -->|Yes| D["Run corresponding suite target"]
  C -->|No| E["Skip during merge transition"]
  F["make <suite>"] --> G{"Entry file exists?"}
  G -->|Yes| D
  G -->|No| H["Fail with missing-suite message"]
Loading

Reviews (1) · Last reviewed commit: "build: include available experiment suit..."

@SamGu-NRX

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 27, 2026 •

Copy link
Copy Markdown
⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@SamGu-NRX

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 27, 2026 •

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

AidenJohnston added a commit that referenced this pull request Sep 27, 2026
build: include available experiment suites in root checks (#90, conflicts resolved)
@AidenJohnston
AidenJohnston merged commit d3c603a into main Sep 27, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 effective changed lines.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants