comments: state the constraint, drop the citation — docs/spec, scripts, .github, cmake and tests (refs #792) - #797
Merged
Merged
Conversation
327 tracker references across 24 spec files — 279 `morph#NNN` and 48 of the `issue #NNN` / bare `#NNN` forms the first pass's grep did not see — replaced by the reasoning they stood in for, per AGENTS.md "Comments and documentation". Most of the diff is rewriting, not deleting. A paragraph whose only support was a ticket number has the support restated: "the same check-then-call shape, elsewhere in `Bridge` — issue #489" becomes the four dispositions and why each site can or cannot take the lifetime gate; backend.md's "What was wrong with the old shape" becomes "Why one bind virtual and not four", which argues the alternative's two costs in the present tense instead of narrating its removal. Every measured block stays whole: executor.md's strand-recycling allocation table, security.md's `gai_strerror` mapping evidence, testing_strategy.md's 302x polling-step swing, registry.md's transparent-key census, backend.md's `ActionCall` round-trip figures. Past-tense narration around them is recast as the condition the number belongs to, and the revisions the measurements were taken on come out with the ticket numbers. backend.md's "Migration status" table — seven landed pull requests — is gone; what it carried that is still true (default `bindModel` routes to the synchronous verb, the `parkIfInFrame` arm is measured-unreachable and kept anyway, the `inlineExecutor()` choice) is restated as current behaviour. `UAX #44` in forms.md is a Unicode standard, not a tracker, and stays. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VptDWG2fKr2vBnLSJcgzgW
…ot for `examples/forge/` held one tracked file, a rung-8 design annex. It is not in `examples/rungs.txt` — which that file calls "the single authority" for rungs — and has no `CMakeLists.txt`, the only directory under `examples/` with neither. Its own header gates it on a decision that has passed: "building the product phases is a post-rung-4 decision", and kanban is rung 4 and shipped. Its four referrers in `examples/LADDER.md` are repaired rather than left dangling: the rung-8 table row goes, the annex range becomes 5-7, and the two sentences that named forge in prose are rewritten so they read correctly without it. `examples/kanban/README.md`'s deferral note loses a cross-reference to "forge phase 2" and states the reason directly, and `examples/IMPLEMENTATION.md`'s FTS5 escapee is described by what it is rather than by which rung would have needed it. No other link into that directory remains. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VptDWG2fKr2vBnLSJcgzgW
Every section of `examples/FINDINGS.md` has somewhere else to live, so keeping it is duplication that drifts: what gets recorded is AGENTS.md's filing bar, the promotion rule is AGENTS.md's "same example defect in a second place", triage dispositions are the `triage-issue` skill's verdict labels, and identity and citation are GitHub issues. It also directed readers to `docs/findings/`, which does not exist. Its one live fact was the "Promoted findings" record: five rungs had accumulated seven near-identical copies of the same idempotency check-then-set table, which is why `morph::offline::IReplayLedger` exists. `docs/spec/offline/offline.md` already carried that argument — the rung count, the call-site count, why storage stays app-side, and the conformance suite — so rather than duplicate it, that paragraph gains the five rung names and the history is dropped. Referrers under `examples/` are rewritten rather than unlinked, since most are instructions to a reader: `IMPLEMENTATION.md` and `LADDER.md` point at AGENTS.md's filing bar; `kanban/README.md`, `pastebin/README.md`, `common/wasm_spike/README.md`, `crm/README.md` and `crm`'s `lead_dto.hpp` state the rule they were citing instead of citing it. The pastebin section explaining why a flat finding sequence was abandoned keeps the argument — a number that outlives its target resolves to the wrong thing — and drops the account of the migration. The `docs/superpowers/` references are left alone: those are plan and spec documents recording work as it was done, and a passing mention of the pipeline in a record of the past is not an instruction to anyone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VptDWG2fKr2vBnLSJcgzgW
…s, not which ticket asked for it 356 tracker references across the gate configuration — ci.yml's 132 the largest share — replaced by the constraint each one stood for. The pattern throughout is the same: a comment saying "this exists because morph#NNN" is rewritten to say what goes wrong without the line, which is what a reader editing it needs and what survives the ticket being closed. Every measured block is kept intact and in the present tense: ci.yml's 302x polling-step swing and its parallel-ctest timings, cmake/tsan.supp's second_deadlock_stack line counts and its 0.275s/0.273s cost, the allowlist's five-run branch censuses, compiler_options.cmake's UBSan-recovers demonstration. Past-tense narration around them is recast as the condition the number belongs to. **User-visible strings changed** — text a person reads on a failure, which AGENTS.md's rule covers as much as a comment: - `.github/workflows/ci.yml` — the clang-tidy filter's "findings in your code (morph#753)" warning. - `cmake/DepCache.cmake` — `morph_declare_dep`'s FATAL_ERROR, and the `FETCHCONTENT_SOURCE_DIR_*` cache docstring. - `cmake/morph_add_rung.cmake` — the semicolon-in-journey-name FATAL_ERROR. - `cmake/compiler_options.cmake` — the warning-sentinel FATAL_ERROR, three coverage cache-sharing messages, and the coverage-manifest FATAL_ERROR. - `scripts/check_branch_coverage.py` — the "contributes no branch records" failure. - `scripts/check_install_export.sh`, `scripts/check_ctest_name_collisions.sh`, `scripts/check_coverage_objects.sh`, `scripts/check_coverage_roots.sh`, `scripts/check_catch2_pin.sh` — one message each. - `scripts/test_check_sanitizer_instrumentation.sh` — two failure strings. Two of those are asserted on by a self-test. `check_branch_coverage.py`'s own `--self-test` keyed two cases on the literal `morph#403` appearing in the message it was checking; both now key on `contributes no branch records`, and the self-test passes. That is the failure mode this kind of edit has: a message is a contract with whatever reads it. `scripts/branch_partial_allowlist.json`: every measured figure and every "what would retire this entry" clause stays. What goes is the provenance — CI job ids, branch names, commit SHAs — and the account of one entry being deleted and restored, which is rewritten as what it actually teaches: without `-fprofile-update=atomic` a coverage run can report that disjunct as taken and fail the gate, and the entry is right anyway. Left alone deliberately: the ctest `-E "OomInjector|morph#108"` filter in ci.yml, which selects by test name, and `morph690_fixture_marker` in a sanitizer test fixture. Both are code that contains a ticket-shaped token; renaming them is a behaviour change, not a comment change. The `net audit finding #10` labels in the allowlist are that audit's own vocabulary, not tracker references, and sit beside `#6`, `#7`, `ST1` and `BK2`. Verified: all four workflows parse as YAML; all 14 distinct `scripts/...` paths they name resolve; `branch_partial_allowlist.json` and `scenario/coverage_allowlist.json` parse as JSON; the three edited Python modules parse; and the self-tests of check_branch_coverage, check_sanitizer_instrumentation, check_coverage_profiles, check_coverage_objects, check_ctest_name_collisions, check_coverage_roots, check_automoc_includes and check_tidy_suppression_scope all pass, as does `check_catch2_pin.sh .`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VptDWG2fKr2vBnLSJcgzgW
… for it 457 tracker references across 97 files under `tests/` — 364 `morph#NNN` and 93 of the `issue #NNN` / bare `#NNN` forms — replaced by the constraint each case exists to pin. A test comment that reads "regression coverage for morph#NNN" tells a reader nothing about what breaks if the case is deleted; each is rewritten to say that instead, in the present tense. Measured blocks stay: `bench_dispatch_allocations.cpp`'s 8.06-per-call figure and its 84-process spread, `bench_dispatch_latency.cpp`'s 302x polling swing and its injected-delay table, `test_bridge_lifetime.cpp`'s 0/200-under-ASan against 26/200 unsanitized, `test_quantity.cpp`'s render timings, `tests/CMakeLists.txt`'s load-average sweeps, `.clang-tidy`'s 751-entry database measurement. **User-visible strings changed**, which is the part to review closely: - **21 `TEST_CASE` names** lost a trailing ` (morph#NNN)` — these are ctest entry names as well as text a person reads in a failure report. Nothing in the repository filters on any of them. - One exception, deliberately kept: `attachHandlerAsync's out-of-frame success callback ... (morph#108)` in `test_async_registration.cpp`. `ci.yml`'s clang-asan/clang-tsan legs exclude tests by name with `-E "OomInjector|morph#108"`, so that token is a selector. Renaming it without editing the workflow would silently stop excluding the case and turn both legs red. `tests/oom_injector.cpp`'s comment now says so explicitly. - `test_strand_race.cpp`'s watchdog line, printed to `stderr` from a second thread, no longer prefixes itself with a ticket number. - `tests/compile_checks/forms_dag_budget.cmake` and `demote_interface_includes_selftest.cmake` — three `FATAL_ERROR` messages. - `tests/net/test_socket_server.cpp` — one `FAIL(...)`. Left as they are, and why: Catch2 tags (`[issue26]`, `[morph583]`, …) are identifiers, not prose; the net audit's own `finding #6`/`#7`/`#8`/`#10`/`#11` labels are that audit's vocabulary rather than tracker references, and appear beside `ST1`/`BK2`/`S3` in the same files; and `test_tcp_socket.cpp` quotes a real ctest line verbatim (`Test #1696: ...`) as evidence, not as a citation. clang-format (22.1.8) re-wrapped the lines these edits reflowed; the whole changed set is clean under `--dry-run -Werror`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VptDWG2fKr2vBnLSJcgzgW
Removing comment lines from include/morph shifted the branches the partial-
coverage allowlist pins, and the gate resolves each entry by line first. Seven
of twenty-one entries no longer landed on their recorded source text, so the
coverage leg failed on master and on every branch cut from it -- including two
pull requests that change nothing near these files.
core/backend.hpp 1302 -> 1294
core/remote.hpp 1416 -> 1413
core/remote.hpp 1494 -> 1491
core/bridge.hpp 2041 -> 2012
net/socket_backend.hpp 146 -> 145
net/socket_backend.hpp 158 -> 157
net/socket_backend.hpp 826 -> 823
Only the hints move. Every entry still resolves to the identical source text it
recorded, each match is unique in its file, and no disposition, reason or
measurement is touched -- the gate's complaint was about where to look, not
about what it found there.
Worth noting for whoever next edits comments in a header the allowlist pins:
this is a line-numbered index into files nothing stops anyone reformatting, and
it drifts silently until a coverage run resolves it. The text match is what
makes the repair mechanical rather than a re-audit.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VptDWG2fKr2vBnLSJcgzgW
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The second pass of the repository-wide comment cleanup
#792began, applyingAGENTS.md's "Comments and documentation" rule to everything outside
include/morphandsrc. Plus two owner-requested deletions.1,143 tracker references removed across four trees, counting both the
morph#NNNform the first pass's grep found and theissue #NNN/ bare#NNNforms it did not:morph#NNNdocs/spec/UAX #44)scripts/+.github/+cmake/tests/examples/is not in this PR — see "Where this stops" below.The shape of the change
Most of the diff is rewriting, not deleting: 567 insertions against 586
deletions over 95 files in the
tests/commit, and 819 against 918 indocs/spec/. Removing a citation frees one clause; restating the reasoning itstood for costs about what it saves. The comment share of these trees has not
meaningfully moved, which is the same result the first pass measured on
include/morph(144 lines out of 18,581).Every measured block is kept whole and moved into the present tense. Among
them:
executor.md's strand-recycling allocation table,security.md'sgai_strerrorread-only-mapping evidence,testing_strategy.md's 302xpolling-step swing,
registry.md's transparent-key census,ci.yml'sparallel-ctest timings,
cmake/tsan.supp'ssecond_deadlock_stacklinecounts and its 0.275s/0.273s cost,
branch_partial_allowlist.json's five-runbranch censuses,
bench_dispatch_allocations.cpp's 8.06-per-call figureacross 84 processes,
test_bridge_lifetime.cpp's 0/200-under-ASan against26/200 unsanitized.
Where a paragraph's only support was a ticket number, the support is restated
rather than the paragraph deleted.
backend.md's "What was wrong with the oldshape" becomes "Why one bind virtual and not four", arguing the alternative's
two costs in the present tense;
concurrency_and_lifetimes.md's "the samecheck-then-call shape, elsewhere in
Bridge— issue #489" becomes the fourdispositions and why each site can or cannot take the lifetime gate.
One section is deleted outright rather than restated:
backend.md's"Migration status" table, seven landed pull requests. What it carried that is
still true — the default
bindModelroutes to the synchronous verb, theparkIfInFramearm is measured-unreachable and kept anyway, theinlineExecutor()choice — is restated as current behaviour.User-visible strings
Text a person reads on a failure is in scope, and this is the part worth
reviewing closely.
TEST_CASEnames lost a trailing(morph#NNN). These are ctestentry names as well as prose. Nothing in the repository filters on any of
them.
attachHandlerAsync's out-of-frame success callback ... (morph#108).ci.yml's clang-asan and clang-tsan legs excludetests by name with
-E "OomInjector|morph#108", so that token is aselector — renaming it without editing the workflow would silently stop
excluding the case and turn both legs red.
tests/oom_injector.cpp'scomment now says so explicitly instead of citing the ticket.
ci.yml's clang-tidy filter warning;DepCache.cmake'sFATAL_ERRORandits
FETCHCONTENT_SOURCE_DIR_*cache docstring;morph_add_rung.cmake'sjourney-name
FATAL_ERROR;compiler_options.cmake's warning-sentinelFATAL_ERROR, three coverage cache-sharing messages and thecoverage-manifest
FATAL_ERROR; one message each incheck_install_export.sh,check_ctest_name_collisions.sh,check_coverage_objects.sh,check_coverage_roots.sh,check_catch2_pin.sh; two intest_check_sanitizer_instrumentation.sh;three
FATAL_ERRORs acrosstests/compile_checks/; oneFAIL(...)intest_socket_server.cpp; andtest_strand_race.cpp's watchdog line,printed to
stderrfrom a second thread.One of those broke a self-test, and that is the finding worth carrying
out of this PR.
scripts/check_branch_coverage.py's own--self-testasserted on the literal
morph#403appearing in the message it waschecking — two cases. Both now key on
contributes no branch records, andthe self-test passes. A user-visible message is a contract with whatever
reads it.
Two deletions
examples/forge/— one tracked file, a rung-8 design annex. It is not inexamples/rungs.txt, which that file calls "the single authority" for rungs,and has no
CMakeLists.txt: the only directory underexamples/withneither. Its own header gates it on a decision that has passed. Its four
LADDER.mdreferrers are repaired rather than left dangling, and two furtherprose mentions in
kanban/README.mdandIMPLEMENTATION.mdnow state whatthey meant instead of pointing at it.
examples/FINDINGS.md— every section has a live home elsewhere(AGENTS.md's filing bar, AGENTS.md's promotion rule, the
triage-issueskill's verdict labels, GitHub issues), and it directed readers to
docs/findings/, which does not exist. Its one live fact was whymorph::offline::IReplayLedgerexists.docs/spec/offline/offline.mdalready carried that argument — the rung count, the call-site count, why
storage stays app-side, the conformance suite — so rather than duplicate it,
that paragraph gains the five rung names and the pipeline prose is dropped.
Eight referrers under
examples/are rewritten as instructions rather thanunlinked; the
docs/superpowers/mentions are left alone, being records ofpast work rather than instructions to anyone.
What is deliberately left
[issue26],[morph583],[morph497]) — identifiers, notprose.
finding #6/#7/#8/#10/#11labels — that audit'sown vocabulary, sitting beside
ST1,BK2andS3in the same files, nottracker references.
morph690_fixture_markerin a sanitizer test fixture, and the-E "OomInjector|morph#108"filter itself — code.test_tcp_socket.cpp'sTest #1696: ...— a real ctest line quotedverbatim as evidence.
UAX #44informs.md— a Unicode standard.Where this stops
examples/is untouched: 466morph#NNNplus 57 other-form referencesremain there. It was last in the order deliberately, and the budget ran out at
four trees.
examples/kanban/gui/andgui_lib/are in any case carved outwhile another lane fixes a layout defect in them.
Gates
clang-format --dry-run -Werror(22.1.8), 84 changed C++ filesctest,clang-debug+ NET + OFFLINE_SQLITE--target doc,WARN_AS_ERROR = FAIL_ON_WARNINGSclang-tidy-diffoverorigin/master...HEADscripts/...path a workflow names resolvesbranch_partial_allowlist.json,scenario/coverage_allowlist.jsoncheck_branch_coverage,check_sanitizer_instrumentation,check_coverage_profiles,check_coverage_objects,check_ctest_name_collisions,check_coverage_roots,check_automoc_includes,check_tidy_suppression_scopeall pass;check_catch2_pin.sh .agrees on all 9 assertionsOn the clang-tidy run: the diff's 85 C++ sections were filtered the way
ci.yml's own filter step filters them — three sources this configure buildsno translation unit for were dropped and named
(
client_only_facade_no_model_header.cpp,forms_dag_probe.cpp, thetests/lint/moc fixture), leaving 82. The database was the wide one, 751entries, the same width CI's clang-tidy job produces.
🤖 Generated with Claude Code
https://claude.ai/code/session_01VptDWG2fKr2vBnLSJcgzgW