fix: strip history-log commentary from TemporalCoordinationDetection#361
Merged
fix: strip history-log commentary from TemporalCoordinationDetection#361
Conversation
Aaron's correction: "comments should not read like history, what
use is this to a future maintainer? Code comments should explain
the code not read like some history log ... code is not a history
file ... there should be existing lint hygiene for that."
GOVERNANCE §2 + CLAUDE.md both say the same thing: docs read as
current state, not history. This file had 27 occurrences of
graduation / ferry / attribution / Otto-NNN / "Provenance:" /
"per correction #N" spread across the module header and six
function docs. Every occurrence is process metadata — who wrote
it, which round shipped it, which absorb it traces to — none of
which a future maintainer reading the function body cares about.
Kept (explains code):
* math: ranges, Euler identity, atan2 + IEEE-754 signed zero,
epsilon-floor rationale
* complementarity: PLV vs cross-correlation, magnitude vs
offset, same-phase vs anti-phase distinction
* input contracts: None semantics, radians, wrapping-
convention-agnostic
* composition guidance: prefer phaseLockingWithOffset for
single-pass; burstAlignment is pair-wise, node-set
generalisation belongs elsewhere
Removed (history):
* module-header "Aaron's design / Amara's formalization /
Attribution / Scope of this first graduation"
* per-function "Provenance: ... N-th courier ferry ...
Nth graduation under the Otto-105 cadence"
* "Per Amara 18th-ferry correction #6" prefix (the argument
it introduced is preserved verbatim, just without the prefix)
* broken inline-code span across newlines in the removed
ferry-path (same class as #348's fix)
37 TCD tests pass. Build clean, 0 warnings. No code bodies
changed — this is a doc-comment-only cleanup. Net -64 lines.
Follow-up: factory-wide audit + lint for the same class of
violation (row landing next tick per queue-saturation
discipline).
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 74ae5437d1
ℹ️ 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".
There was a problem hiding this comment.
Pull request overview
This PR cleans up TemporalCoordinationDetection by removing process/history-style prose from XML doc comments and keeping only code-relevant documentation for the temporal coordination primitives.
Changes:
- Rewrites the module header doc to describe the primitives and their intended composition without provenance/history metadata.
- Tightens and reflows several function doc comments (cross-correlation profile, phase helpers, PLV/offset, burst alignment) to be current-state documentation.
This was referenced Apr 24, 2026
Thread 59Y_Np (Codex P2) + thread 59Y_st (Copilot P1) — same
finding: the module-level summary claimed "All entry points
return Option-wrapped values" and listed "length-mismatched"
as a None condition, but:
* crossCorrelationProfile / significantLags / burstAlignment
return plain arrays, not Option-wrapped
* crossCorrelation tolerates mismatched lengths — it operates
over the overlap window at the given lag
Split the module doc into two return-shape families (single-
value primitives that return Option, profile/array primitives
that return arrays) and added an explicit note on length
semantics: crossCorrelation tolerates mismatch, phase-pair
primitives (PLV / meanPhaseOffset / phaseLockingWithOffset)
require equal lengths.
Thread 59Y_tT (Copilot P2) — meanPhaseOffset doc wording:
"vector whose magnitude is phaseLockingValue" implied the
function IS the magnitude. Corrected to "whose magnitude is
the PLV (i.e. the value returned by phaseLockingValue on the
same inputs)".
All three are doc-comment-only corrections. 37 TCD tests pass;
build clean.
AceHack
added a commit
that referenced
this pull request
Apr 24, 2026
) * loop: tick-history row — Otto-219..221 drain + Otto-220 correction Three-PR drain (#348 clean, #340 merged, #361 opened) plus Aaron's Otto-220 correction on code-comments-vs-history absorbed into a durable memory + PR #361 fix landed against main. No code changes; append-only history row per docs/AUTONOMOUS-LOOP.md end-of-tick checklist. * fix(#362): typo "don not integrate" -> "don't integrate" Copilot P2 thread 59ZEMT — one-character typo fix in the quoted-text portion of the Otto-219..221 tick row's Observation 4. The surrounding quote survives; only the typo shifts.
AceHack
added a commit
that referenced
this pull request
Apr 24, 2026
Aaron's durable correction to factory-authored code comments: "Code comments should explain the code not read like some history log ... there should be existing lint hygiene for that." One file-level cleanup (PR #361 on TemporalCoordinationDetection) drains a single offender; the class of bug is structural and recurs on every new primitive unless caught automatically. This PR ships the structural enforcement. What it does: * Scans src/**, tests/**, bench/**, tools/** source files (.fs/.cs/.sh/.ts) for high-signal history-lineage tokens inside doc-comment lines (leading ///, //, # — not shebang) * Three modes: --list (advisory), --fail-any (strict), and default-check (fails only on violations not in baseline) * Baseline: tools/lint/doc-comment-history-audit.baseline captures the 105 existing violations across 19 files so the lint lands non-blocking; subsequent cleanup PRs drain it What it does NOT do: * Scan docs/**, openspec/**, memory/** — those legitimately carry history and are out of scope * Block existing debt — baseline pins current state * Wire into CI yet — Aaron decides if/when to fail pre-commit or CI jobs on new violations; shipping the tool first so the discipline is at least measurable Token list (TOKEN_PATTERN in the script): Otto-\d+ / Amara / Aaron / ferry / courier / graduation / Provenance: / Attribution: Top offenders in the baseline (cleanup follow-ups, likely one PR each): src/Core/Graph.fs 34 violations src/Core/TemporalCoordinationDetection 25 violations (PR #361) src/Core/Veridicality.fs 14 violations src/Core/RobustStats.fs 10 violations Rationale: memory/feedback_code_comments_explain_code_not_history_otto_220_2026_04_24.md
AceHack
added a commit
that referenced
this pull request
Apr 24, 2026
…to-220 (#363) * tools: doc-comment history-audit lint — structural enforcement Aaron's durable correction to factory-authored code comments: "Code comments should explain the code not read like some history log ... there should be existing lint hygiene for that." One file-level cleanup (PR #361 on TemporalCoordinationDetection) drains a single offender; the class of bug is structural and recurs on every new primitive unless caught automatically. This PR ships the structural enforcement. What it does: * Scans src/**, tests/**, bench/**, tools/** source files (.fs/.cs/.sh/.ts) for high-signal history-lineage tokens inside doc-comment lines (leading ///, //, # — not shebang) * Three modes: --list (advisory), --fail-any (strict), and default-check (fails only on violations not in baseline) * Baseline: tools/lint/doc-comment-history-audit.baseline captures the 105 existing violations across 19 files so the lint lands non-blocking; subsequent cleanup PRs drain it What it does NOT do: * Scan docs/**, openspec/**, memory/** — those legitimately carry history and are out of scope * Block existing debt — baseline pins current state * Wire into CI yet — Aaron decides if/when to fail pre-commit or CI jobs on new violations; shipping the tool first so the discipline is at least measurable Token list (TOKEN_PATTERN in the script): Otto-\d+ / Amara / Aaron / ferry / courier / graduation / Provenance: / Attribution: Top offenders in the baseline (cleanup follow-ups, likely one PR each): src/Core/Graph.fs 34 violations src/Core/TemporalCoordinationDetection 25 violations (PR #361) src/Core/Veridicality.fs 14 violations src/Core/RobustStats.fs 10 violations Rationale: memory/feedback_code_comments_explain_code_not_history_otto_220_2026_04_24.md * fix(#363): 3 review threads — ERE word-boundary + header accuracy + per-token baseline - PRRT_kwDOSF9kNM59ZMZ- (P0, line 73): TOKEN_PATTERN previously used `\b` word-boundary anchors under `grep -E` (ERE); BSD grep on macOS treats `\b` as a literal `b`, silently missing matches. Reworked collect_violations() to do extraction + token matching inside awk, using explicit `[^A-Za-z0-9_]` boundary checks via match() / substr(). Portable across GNU awk and BSD awk; no grep dependency for the match step. - PRRT_kwDOSF9kNM59ZMau (line 36): header doc over-promised by claiming `<!--` was a recognised comment start; the awk extractor never handled it and scanned file types (.fs, .cs, .sh, .ts) rarely use it. Removed `<!--` from the doc comment. - PRRT_kwDOSF9kNM59ZMd5 (P2, line 113): previous extractor emitted only the first token per line, so baselined lines that gained additional forbidden tokens silently passed. Awk loop now collects all token matches per line, sorts + dedupes them, and emits `file:line:token1,token2,...`. Any new token on a baselined line changes the record and is caught by the baseline comparison. Baseline regenerated — 82 entries (down from 105; the earlier count reflected stale baseline + BSD-grep under-counting masking the actual corpus).
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
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.
Summary
Aaron (Otto-220) flagged that a doc comment I added on PR #340
read like a history log, not like code documentation:
The comment in question was the "Provenance: primitive from the
human maintainer's differentiable firefly-network design,
formalized in an external AI collaborator's 11th courier ferry
... Third graduation under the Otto-105 cadence." block on
phaseLockingValue— pure process metadata, zero code value.On re-reading the whole file, the same class of violation
appeared 27 times across the module header and six function
docs. Leaving the other 26 would mean relitigating the same
correction on every subsequent edit. This PR strips them all.
What stays (explains the code)
[0.0, 1.0]+[-pi, pi]ranges, Euler identity, atan2behaviour under IEEE-754 signed-zero, epsilon-floor rationale
(including the same-phase vs anti-phase distinction — the
argument is preserved, just without the "per correction Round 30 — threat-model elevation (nation-state + supply-chain) #6"
prefix)
Nonesemantics, radians, wrapping-convention-agnostic
phaseLockingWithOffsetforsingle-pass;
burstAlignmentis pair-wise, node-setgeneralisation belongs elsewhere
What goes (history)
Attribution / Scope of this first graduation"
graduation under the Otto-105 cadence"
path (same CommonMark bug class as PR backlog: Otto-168 Frontier naming conflict with OpenAI Frontier — rename factory UI #348's fix)
Verification
dotnet build -c Release src/Core/Core.fsproj— 0 Warning(s)dotnet test ... --filter 'FullyQualifiedName~TemporalCoordinationDetection'— 37 passed, 0 failed
grep -c 'graduation\|ferry\|Amara\|Aaron\|Otto-\|Attribution\|Provenance' src/Core/TemporalCoordinationDetection.fs— 0 after fix (was 27)
Test plan
dotnet build -c Release src/Core/Core.fsprojcleandotnet testTCD filter — 37/37 passmetadata rather than load-bearing math
Follow-up
Aaron asked about lint enforcement. Filing a BACKLOG row next
tick for: (a) factory-wide audit of
src/Core/**/*.fsfor thesame class of violation, (b) a markdownlint/custom-lint rule
that flags
///lines containinggraduation|ferry|Otto-\d| Amara|Aaron|Provenance:|Attribution:|"N-th"|"correction #"in committed source — violations get flagged at pre-commit so
the class of bug stops recurring.