docs(research): 36 cohort test artifacts exist, 9 run in CI (#342) - #436
Conversation
Signed-off-by: tucktuck101 <jeffreytaylorrobertson@gmail.com>
serina-mcfall
left a comment
There was a problem hiding this comment.
Blocker — the headline "31" is 32, and the note's own table says so
Using the note's own definition (a test_*.py or test-*.sh file under launchpad/), at the pinned SHA:
$ git ls-tree -r --name-only f75085d5c -- launchpad | grep -cE '/test_[^/]*\.py$'
18
$ git ls-tree -r --name-only f75085d5c -- launchpad | grep -cE '/test-[^/]*\.sh$'
14
18 + 14 = 32. The note's own coverage table sums the same way — 6 + 13 + 5 + 6 + 1 + 1 = 32. deploy/test-run-guard.sh appears as a row in the table and is then dropped from the total.
The same off-by-one surfaces twice more from the same root:
- line 49 — "26 shell scripts of which 13 are tests." Fourteen match
test-*.sh; 13 is the count forlaunchpad/scriptsalone. - line 13 — "13 production shell scripts." 26 − 14 = 12.
Why this blocks rather than files: 31 is the PR title, the frontmatter description, and the denominator of "6 of 31" (line 90), "25 unrun test artifacts" (line 128), and the criterion-8 phrasing. Those become 6 of 32, 26 unrun, 14 shell test scripts. This is exactly the figure that gets lifted into #290 and never re-derived — and #290's criterion 8 is being written from it.
Low — .yaml vs .yml
Lines 13 and 163 say "34 YAML files". The count is right; every one of them is .yml. The label will send the next person grepping for the wrong glob.
What is right — and it is nearly all of it
The number that actually matters, "6 run in CI", is correct, and so is essentially every supporting figure. Verified independently:
- Extension totals reproduce exactly:
62 py / 26 sh / 34 yml. - Production Python by directory reproduces exactly: scripts 11, review-agent 24 (21 + 3 in
dimensions/), project-intelligence 6, agents 3 (1 + 2 inthe-professor/tools/) = 44; 44 + 18 = 62. Thelaunchpad/scriptsshell figure of 16 is right too (14 in the directory + 2 intestdata/). - The complete
run:inventory across the fivelaunchpad-*.ymlworkflows reproduces line for line — including the detail that the threediscoverinvocations differ (adr-checkbare at :63,pr-checkwith-t … -vat :100,security-auditnarrowed with-p "test_security_audit*.py"at :56). That is the kind of distinction most inventories flatten. - No workflow references
launchpad/project-intelligence,launchpad/agents, or anylaunchpad/scripts/test-*.sh. The singlelaunchpad/deploymention repo-wide isdocker.yml:142, and it is a comment — exactly as claimed. - The
CONTROLSlist is verbatim: 13 entries, none a test module. suite.pyreferences none of the fivetest_*.pymodules.
Your "Not checked" list is honest and includes the sharpest question in the whole area — whether discover actually collects all six, given that empty discovery reads as a pass. That one is worth its own issue whatever happens to the count.
The fix here is arithmetic, not research. The research is sound.
Reviewed at head f75085d5c. The recount was run by me directly at that SHA.
🤖 Review drafted by Claude Code (claude-opus-5) for @serina-mcfall.
Signed-off-by: tucktuck101 <jeffreytaylorrobertson@gmail.com>
Review panel — two independent reviewers (Fable, gpt-5.6-sol xhigh), consolidatedSummary: Both reviewers independently reached the same verdict: changes needed. The research method is sound, the prior 31→32 headline fix was real, and the note's central contribution (the controls-vs-suites distinction in Blocking
High
Medium
Low
What looks right (independently verified by both reviewers)
Panel: fable + gpt-5.6-sol (codex, xhigh), independent reviews consolidated by the lane; the reviewers converged on the blocking findings from independent recounts. Author is tucktuck101, so this is a comment, not a formal review — needs an external reviewer once addressed. Posted under Jeff's standing authorization. |
serina-mcfall
left a comment
There was a problem hiding this comment.
Re-review — the 31→32 fix landed; two blockers stand, one of them new
Agent-authored under @serina-mcfall's instruction. Per launchpad/AGENTS.md:165 —
"Draft everything. Approve nothing." — this raises concerns and clears none.
The original blocker is genuinely fixed. The headline, frontmatter and coverage table all read
32 now, and the frontmatter says "14 shell test scripts". That was the arithmetic, and it is right.
Blocker 1 — the line the previous review named explicitly is still unfixed, and now contradicts the note's own frontmatter
The previous review listed three occurrences of the off-by-one and quoted this one by line number.
It is unchanged:
$ git show origin/research/342-cohort-executable-inventory:launchpad/Research/342-cohort-executable-inventory.md | sed -n '49p'
**Totals: 44 production Python modules, 18 Python test files, 26 shell scripts of which 13 are
My own recount at this PR's head:
$ git ls-tree -r --name-only origin/research/342-cohort-executable-inventory -- launchpad | grep -cE '/test-[^/]*\.sh$'
14
$ ... | sed 's|/[^/]*$||' | sort | uniq -c
1 launchpad/deploy
13 launchpad/scripts
14, not 13. The dropped artifact is launchpad/deploy/test-run-guard.sh — the entry the note's own
security paragraph calls the highest-consequence unrun item. Line 49 now says 13 while line 2 says
14, in one document. :154 carries the same unscoped 13 ("the 13 test-*.sh scripts"); :20 is
fine because it scopes to launchpad/scripts/.
Blocker 2 — the note pins no revision, and its counts are already wrong on today's launchpad
The note states its figures in the present tense — "32 test artifacts exist under launchpad/.
Six of them run in CI" — and I can find no SHA pin or as-of date anywhere in it. Measured at
origin/launchpad today:
$ git ls-tree -r --name-only origin/launchpad -- .github/workflows | grep launchpad
.github/workflows/launchpad-adr-check.yml
.github/workflows/launchpad-agents-tests.yml <- sixth workflow, not in this branch
.github/workflows/launchpad-issue-check.yml
.github/workflows/launchpad-pr-check.yml
.github/workflows/launchpad-review-agent-controls.yml
.github/workflows/launchpad-security-audit.yml
$ git ls-tree -r --name-only origin/launchpad -- launchpad/review-agent | grep -cE '/test_[^/]*\.py$'
7
$ git ls-tree -r --name-only origin/launchpad -- launchpad/agents | grep -E 'test_.*\.py$'
launchpad/agents/test_goose_config.py
launchpad/agents/test_project_pack.py
$ git ls-tree -r --name-only origin/launchpad -- launchpad | grep -cE '/test_[^/]*\.py$' # 22
$ git ls-tree -r --name-only origin/launchpad -- launchpad | grep -cE '/test-[^/]*\.sh$' # 14
So on trunk today it is 36 test artifacts, not 32; launchpad/review-agent has 7 suites,
not 5; launchpad/agents has 2 test files, not 1 — and the coverage-table row
`launchpad/agents/test_*.py` | 1 | **no** | no workflow mentions the directory (#270) is
affirmatively false, because launchpad-agents-tests.yml discovers and runs exactly that directory
(:71 suite = unittest.defaultTestLoader.discover("launchpad/agents", pattern="test_*.py")).
This blocks because the note is the denominator #290's criterion 3 will lift, and it would land
stating a coverage gap that has since been closed. Either fix closes it: add an explicit
"as of commit <sha>" pin and leave the figures as dated evidence, or rebase and re-derive against
the new base. The pin is the cheaper and more durable of the two, and it is the convention the
sibling notes on #405/#413/#374 already adopted.
What is right
The controls-vs-suites distinction in launchpad/review-agent/run_controls.py is the note's real
contribution and it is unaffected by either blocker — the CONTROLS list is verbatim, 13 entries,
none of them a test module. The extension totals, the per-directory production Python breakdown,
the complete run: inventory across the workflows including the three differing discover
invocations, and the observation that no workflow runs any launchpad/ shell test all still hold.
The "Not checked" section remains the sharpest in this batch — the empty-discovery hazard it names
is precisely what the new agents workflow's own guard at :60-71 was written to catch.
The research is sound. It is the pinning discipline and one arithmetic line.
Reviewed at head b3f9475/c812a796e. Every count above was run by me directly at the stated ref, not taken on report.
🤖 Review drafted by Claude Code (claude-opus-5) for @serina-mcfall.
|
Correction to the footer of my review just above: the head reviewed is |
…local Signed-off-by: tucktuck101 <jeffreytaylorrobertson@gmail.com>
…un in CI (#342) Re-derives every count at the merged base tip and pins the note to it: the new launchpad-agents-tests.yml workflow moves the agents suites into CI, and four new test files land since f75085d. Adds the mechanical per-executable appendix issue #342's DoD asks for (mode + shebang + extension + playbooks), which surfaces deploy/archived/deploy; classifies the 34 .yml files with arguable boundaries recorded as arguable; un-elides the derivation pipeline; fixes the three surviving 13-vs-14 shell-test sites and criterion 8 (14 shell + 13 Python = 27 unrun). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: tucktuck101 <jeffreytaylorrobertson@gmail.com>
|
Re-review requested — pushed afc4dea (base Panel blocker 1 (counts stale vs. moved base; no SHA pin; elided pipeline) — fixed, with one correction to the finding itself. The note now opens with an explicit pin to Serina's blocker + panel blocker 2 (surviving 13-vs-14 sites) — fixed. The three sites that survived c812a79 ( Panel high (per-executable inventory, extensionless executable) — fixed. New appendix: a 103-row per-file table derived mechanically (extension ∪ git mode 100755 ∪ shebang ∪ Ansible playbook), with test name-match and CI columns — this is what surfaces Panel medium (PR body + standing #342 comment) — fixed. PR title and body re-derived at the pin (title now "36 … 9"); a superseding correction is posted on #342 (issuecomment-5389629505) with the corrected table and the pinned SHA. Lows — fixed. The "34 yaml" block was hand-edited output — it's now the real command and real output, and the note states explicitly that all 34 are One deliberate scope note: the empty-discovery hazard the first revision flagged is now half-closed upstream — |
serina-mcfall
left a comment
There was a problem hiding this comment.
Both blockers verified fixed — dismissing the change-requests
Agent-authored under @serina-mcfall's instruction. Per launchpad/AGENTS.md:165 — "Draft everything. Approve nothing." — this clears the two blockers I raised and does not approve the PR. It still needs an approving review from someone else.
Head reviewed: afc4deaf3.
Blocker 1 — the unscoped 13 — FIXED
The previous review quoted line 49 by number and it was unchanged at that point. It is fixed now:
$ git show pr436chk:launchpad/Research/342-cohort-executable-inventory.md | grep -nE "26 shell scripts of which"
66:**Totals: 48 production Python modules, 22 Python test files, 26 shell scripts of which 14 are
14, not 13. And line 32 now carries the breakdown that makes the number self-checking rather than merely correct:
32:1. **All 14 `test-*.sh` scripts (13 in `launchpad/scripts/`, 1 in `launchpad/deploy/`) are run by
That is the right shape — the 13 was never wrong, it was unscoped, and naming both directories removes the ambiguity instead of just changing a digit.
I also checked the other occurrence the review flagged. No unscoped 13 survives anywhere:
$ git show pr436chk:...342-cohort-executable-inventory.md | grep -nE "13 .test-\*\.sh|which 13"
(no output)
Blocker 2 — no revision pin — FIXED, and the pin holds
Line 8 now states:
All counts in this document are as of commit
132f921ace6be6d391c03e134b304ddea3ecccf0
A pin is only worth having if its numbers reconcile at it, so I checked that rather than the pin's presence:
$ git cat-file -t 132f921ace6be6d391c03e134b304ddea3ecccf0
commit
$ git ls-tree -r --name-only 132f921ac -- launchpad | grep -cE '/test_[^/]*\.py$'
22
$ git ls-tree -r --name-only 132f921ac -- launchpad | grep -cE '/test-[^/]*\.sh$'
14
22 + 14 = 36, exactly what the note claims. The commit exists, and every headline figure derives from it. A pin whose counts did not reconcile would have been worse than no pin at all — this one is sound.
What is genuinely good about this revision
The fix went further than the finding required. The original blocker was an arithmetic error — 31 where the note's own table summed to 32. Rather than patch the digit, this revision re-derived every count at a pinned commit, which is why the figures moved again (32 → 36) and why they now reconcile. That is the difference between correcting a number and fixing the reason it was wrong.
The launchpad/deploy/test-run-guard.sh entry — the one dropped from the total, and the one the note's own security section calls the highest-consequence unrun item — is now counted and named.
What I have not checked
I verified the two blockers and the arithmetic behind them. I did not re-review the rest of the note at this head: the CI-invocation inventory, the coverage table's per-row claims, or the appendix classification. The previous review confirmed those at an earlier head, and the recount has since moved several figures, so a fresh pass over the unchanged sections is not something I can claim to have done.
Also unchecked, and still the sharpest open question in the note's own "not checked" list: whether discover actually collects all the suites it is pointed at — empty discovery reads as a pass.
Dismissing review 5003891553 and 4998595585. This is not an approval — reviewDecision returns to REVIEW_REQUIRED and a human still has to sign off.
🤖 Review drafted by Claude Code (claude-opus-5) for @serina-mcfall.
serina-mcfall
left a comment
There was a problem hiding this comment.
Approved. Both blockers verified fixed; counts reconcile at the pinned commit.
Summary
Answers #342 with the denominator criterion 3 was missing — re-derived at pinned base
132f921ace6be6d391c03e134b304ddea3ecccf0(2026-08-24): 48 production Python modules, 12 production shell scripts, one extensionless Bash entry point (deploy/archived/deploy) and 34.ymlfiles underlaunchpad/, guarded by 36 test artifacts (22 Python test files, 14test-*.sh). CI invokes exactly three things and covers 9 of the 36. The two unnamed gaps are the 14 shell test scripts, which no workflow runs, andreview-agent's seven unittest suites, which are easy to confuse with its containment controls. A mechanical per-executable appendix (mode + shebang + extension + playbooks, 103 rows) makes the denominator per-file, and classifies the 34.ymlfiles with arguable boundaries recorded as arguable.Related issue
Closes #342
Issue type
Task
Agent provenance
Objective
Add
launchpad/Research/342-cohort-executable-inventory.mdenumerating cohort-owned executables underlaunchpad/and, per test artifact, whether any CI workflow runs it.Impacted components
launchpad/Research/342-cohort-executable-inventory.md
Approach and rejected alternatives
Enumerated from
git ls-filesso the method is mechanical and reproducible — extension histogram first, then a second pass over git file mode100755+ shebang + Ansible playbooks, which is what catches the extensionlessdeploy/archived/deploy— then listed every test-relevantrun:line across all sixlaunchpad-*.ymlworkflows and matched the two sets. #342 asked specifically for a reproducible mechanical method rather than a remembered list, because the failure mode is a forgotten directory.Rejected trusting the workflow names.
launchpad-review-agent-controls.ymlsounds like it runs the review agent's tests; readingrun_controls.py's hardcodedCONTROLSlist shows it runs thirteencheck_*.py/suite.pyentries and none of the seventest_*.pysuites. That distinction is the main thing this document contributes, and a name-level reading would have missed it.Verification
Commands run at
132f921ac(merged into this branch):Raw output:
Not verified
Whether the 14
test-*.shscripts pass. I inventoried them; I ran none. They may be green, stale or broken, and that changes the cost of wiring them in considerably.Whether
unittest discover -s launchpad/scriptsactually collects all seven Python test files. It should by naming convention, but I did not run it and read the count.launchpad-agents-tests.ymlnow guards against empty discovery forlaunchpad/agents(counting collected cases, not files); the threelaunchpad/scriptsdiscover invocations have no such guard.Whether review-agent's 13
check_*.pycontrols subsume what its seventest_*.pysuites assert. I established they are different invocations, not that the coverage is disjoint.Executable-boundary edge cases are recorded, not settled —
bin/lefthook(a cohort divergence under ADR-0017, executable, outsidelaunchpad/), the five ADR-0005 deployment files (cohort-owned, not executable), and Ansible role tasks/handlers (executed only by inclusion). The 34.ymlfiles are now classified in the note's appendix on exactly those terms.Upstream's
scripts/directory is excluded, though the fork's CI depends on several of those scripts.Security implications
None from the document. One observation for reviewers:
launchpad/deploy/'s only test artifact,test-run-guard.sh, is not run by any workflow — and the deploy scripts are the code that touches the live relay host. That is the highest-consequence entry in the unrun column, together withdeploy/archived/deploy, the archived extensionless deploy driver with no test at all.Escalations
launchpad/scriptsand (since8f4293543)launchpad/agentsthe full suites run on every pull request, plusmutation_harness.pyfor scripts. And my own earlier sweep comment on prd: the cohort test suite — what we test that upstream doesn't #290 implied the review agent's suites run in CI — they do not. Editing the PRD is not mine to do.launchpad/agentsgap (task: run launchpad/agents Python test suites in CI #270) has meanwhile closed, so the named gap and the real gap barely overlap.CONTROLS, or whether the controls already suffice, is a call for whoever owns that code.