Repository navigation
Enforce sparse source comments with Aontu policy controls - #44
Conversation
Adopt Aontu ADR-032, clean existing source comments, and enforce the policy through tests, CI and pre-push hooks. Preserve executable directives and move durable rationale into Markdown. Validation: make comments comments-test (9 tests); git diff --check. Package and corpus validation is recorded in the PR description.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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[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: ea9d6350b4
ℹ️ 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".
| @@ -0,0 +1,13 @@ | |||
| name: Source comment policy | |||
There was a problem hiding this comment.
This commit adds a workflow directly, but the repository requires workflow changes from agent sessions to arrive as a git format-patch plus an applying README under patch/, because the supported credentials cannot push .github/workflows/. Move this addition to that delivery mechanism; otherwise the changeset cannot be delivered through the supported path.
AGENTS.md reference: AGENTS.md:L231-L238
Useful? React with 👍 / 👎.
| ] | ||
|
|
||
| const PATH_RE = /\b((?:[\w.-]+\/)+[\w.-]+\.(?:ts|go|md|tsv|cjs|mjs|js|json|aontu|aon|abnf|yml|sh))\b/g | ||
| const ADR_RE = /\bADR-(\d{3})\b/g |
There was a problem hiding this comment.
When a source comment cites this repository's ADR-1 through ADR-4 form, this three-digit-only regex ignores it completely. In addition, adrs() indexes a nonexistent root ADR.md rather than docs/ADR.md, so merely widening this regex would mark every valid citation stale. As a result, the new stale-adr rule cannot validate any actual repository ADR citation; accept unpadded IDs and index the real ADR file.
AGENTS.md reference: AGENTS.md:L344-L348
Useful? React with 👍 / 👎.
| const regexStart = lang === 'ts' && (!/[\w$)\]]/.test(prev) || | ||
| /\b(?:return|throw|yield|case)\s*$/.test(text.slice(Math.max(0,i-30),i))) | ||
| if (c === '/' && regexStart) { |
There was a problem hiding this comment.
For valid TypeScript such as const z = x++ / y // TODO: fix this, prev is +, so the division slash is classified as the start of a regex and the first slash of the real line comment is consumed as its closing delimiter. The lexer consequently reports no comment and the prohibited TODO passes the gate; distinguish postfix ++/-- expressions from regex-start contexts.
AGENTS.md reference: AGENTS.md:L344-L348
Useful? React with 👍 / 👎.
| const DIRECTIVE_RE = | ||
| /^(#!|\/\/\s*(EJECT-(?:START|END)|<\[|#[A-Z][\w-]*\b|go:|nolint|@ts-|eslint|prettier|c8 |istanbul|coverage:ignore|covergate:allow|\/\s*<reference)|\/\*\s*([@#]__PURE__|[@#]__NO_SIDE_EFFECTS__|node:coverage|eslint|c8|istanbul|prettier))/ |
There was a problem hiding this comment.
The generic #[A-Z][\w-]* alternative exempts every comment beginning with an uppercase hash token, not just executable markers such as #SecretsImport. Consequently ordinary violations such as // #TODO: fix this or // #NARRATIVE We fixed the old code bypass every rule and do not count toward density; enumerate the supported markers or otherwise validate that the token is an actual tooling directive.
AGENTS.md reference: AGENTS.md:L344-L348
Useful? React with 👍 / 👎.
| }, | ||
| ] | ||
|
|
||
| const PATH_RE = /\b((?:[\w.-]+\/)+[\w.-]+\.(?:ts|go|md|tsv|cjs|mjs|js|json|aontu|aon|abnf|yml|sh))\b/g |
There was a problem hiding this comment.
The path matcher omits extensions used by most ports, including .py, .rb, .php, .java, .cs, and others. A checked comment such as // See python/voxgig_plugin/no_such_file.py therefore produces no stale-path finding, while an equivalent missing .go path does, allowing stale cross-port references to remain green; path validation should not depend on this partial target-extension allowlist.
AGENTS.md reference: AGENTS.md:L344-L348
Useful? React with 👍 / 👎.
| const model = /\.(aon|aontu)$/.test(rel) | ||
| return SOURCE_EXTS.some((ext) => rel.endsWith(ext)) && | ||
| !rel.split('/').some((part) => SKIP_DIRS.has(part) && !(model && part === 'target' && /(^|\/)model\/target\//.test(rel))) && |
There was a problem hiding this comment.
For an authored file such as model/target/implementation.ts, model is false because it is based on the extension, so the target directory exclusion drops the file even though .ts is otherwise in scope. The same happens to .go and .rs implementation helpers placed beside generated models, leaving their comments completely unchecked; make the model/target/ exception depend on the path and reserve excludeModels for generated model files.
AGENTS.md reference: AGENTS.md:L344-L348
Useful? React with 👍 / 👎.
| const density = scan.codeLines === 0 ? 0 : commentLines / scan.codeLines | ||
| if (commentLines > DENSITY_FLOOR_LINES && density > MAX_DENSITY) { |
There was a problem hiding this comment.
When a source file has no code lines, this assigns density zero regardless of how many comments it contains. A file with nine or more one-line comments separated by blank lines therefore avoids both long-block and dense-file and can grow as arbitrary prose while the gate remains green; once the comment floor is exceeded, zero code lines should be treated as a density violation rather than 0%.
AGENTS.md reference: AGENTS.md:L344-L348
Useful? React with 👍 / 👎.
| const CODE_SHAPED_RE = | ||
| /^\s*(?:(?:const|let|var|func|function|type|class|interface|import|export|package|return|if|for|while|switch|case|await|throw|new|await)\b(?=[^\n]*[;{}()=[\]])|[\w$.[\]]+\s*(?::=|\+=|=[^=])|[\w$.]+\([^)]*\)\s*[;{]?\s*$|[})\]];?\s*$)/ |
There was a problem hiding this comment.
The shared code-shaped pattern includes TypeScript and Go declaration keywords but omits ordinary Rust forms. As a result, comments such as // fn disabled() {}, // pub fn disabled() {}, // struct Disabled {}, and // use crate::foo; produce no commented-code finding even though .rs files are explicitly scanned; add Rust declaration and import forms to the language-aware check.
AGENTS.md reference: AGENTS.md:L344-L348
Useful? React with 👍 / 👎.
1 participant
Adds Aontu ADR-032's sparse-comment policy and removes prohibited prose, stale references and commented-out code. Durable rationale lives in Markdown; license notices, coverage/compiler directives and executable generator markers are preserved.
The standalone checker covers TypeScript, Go, Rust,
.aonand.aontu, including authored models, test corpora and generator templates. It runs through Make, applicable npm test commands, a tracked pre-push hook and a GitHub Actions workflow with pinned actions. Generated goldens and vendored snapshots have explicit exclusions. Regression tests exercise both acceptance and rejection, including CLI and hook failures for new files.Validation:
make comments comments-test: passed, including all 9 control tests.git diff --check: passed.