keep GitHub language stats focused on TypeScript#389
Conversation
The runtime wrappers and maintenance scripts stay in JavaScript, but GitHub's repository language bar should reflect the authored product source instead of auxiliary script formats. This adds Linguist detectability overrides in `.gitattributes` and locks them in the documentation governance suite. Constraint: Wrapper and maintenance scripts must remain directly executable without build pipeline changes Rejected: Convert scripts and wrappers to TypeScript | would widen runtime and packaging scope beyond a metadata-only change Confidence: high Scope-risk: narrow Directive: If a non-TypeScript repo artifact should count toward the language bar later, update `.gitattributes` and `test/documentation.test.ts` together Tested: `git check-attr linguist-detectable -- index.ts scripts/codex.js scripts/check-pack-budget.mjs scripts/test-all-models.sh lib/oauth-success.html`; `npm test -- test/documentation.test.ts`; `npm run lint`; `npm run typecheck`; `npm run build` Not-tested: Full `npm test` remains red on inherited `test/proactive-refresh.test.ts` timeouts reproduced on clean `origin/main`
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
📝 WalkthroughWalkthroughadds a Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Notes for reviewthe test at
no concurrency or logic risks here. straightforward configuration addition. 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/documentation.test.ts`:
- Around line 620-624: The test currently uses substring assertions on the
generated .gitattributes content (variable content) which allows contradictory
rules to coexist; update test/documentation.test.ts to assert the exact override
contract by either (1) asserting full-line matches for each glob using regex
with multiline anchors (e.g., expect(content).toMatch(/^\*\.ts
linguist-detectable$/m) for "*.ts") or (2) parse the content into lines and
assert that for each extension (".ts", ".js", ".mjs", ".sh", ".html") there is
exactly one rule and that it equals the expected token ("linguist-detectable" or
"-linguist-detectable"), and fail if both variants exist; replace the current
expect(content).toContain(...) calls with these stricter checks on the content
variable so the test fails if contradictory rules are introduced.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d3268f61-a3ab-480f-ad52-b0846437cd9e
📒 Files selected for processing (2)
.gitattributestest/documentation.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (1)
test/**
⚙️ CodeRabbit configuration file
tests must stay deterministic and use vitest. demand regression cases that reproduce concurrency bugs, token refresh races, and windows filesystem behavior. reject changes that mock real secrets or skip assertions.
Files:
test/documentation.test.ts
🔇 Additional comments (1)
.gitattributes (1)
1-6: linguist override block is clear and scoped.this rule set is minimal and matches the stated goal, and it is guarded by
test/documentation.test.ts:612-625.
The first pass only checked substring presence, which meant the test could stay green even if conflicting `.gitattributes` rules were added later. This switches that guardrail to an exact normalized line match for the override block. Constraint: Keep the fix scoped to the single actionable CodeRabbit finding on PR #389 Rejected: Add broader parser helpers or extra metadata tests | unnecessary for a five-line override contract Confidence: high Scope-risk: narrow Directive: Keep this assertion exact; if the override block changes, update the expected normalized lines in one place Tested: `npm test -- test/documentation.test.ts`; `npm run lint`; `npm run typecheck` Not-tested: Full `npm test` remains red on inherited `test/proactive-refresh.test.ts` timeouts reproduced earlier on clean `origin/main`
Summary
What Changed
.gitattributesrules that keep*.tsdetectable while marking*.js,*.mjs,*.sh, and*.htmlas non-detectable for repo language statstest/documentation.test.tsassertion so the Linguist override contract stays explicit in repo governance coverageValidation
npm run lintnpm run typechecknpm testnpm test -- test/documentation.test.tsnpm run buildDocs and Governance Checklist
docs/getting-started.mdupdated (if onboarding flow changed)docs/features.mdupdated (if capability surface changed)docs/reference/*pages updated (if commands/settings/paths changed)docs/upgrade.mdupdated (if migration behavior changed)SECURITY.mdandCONTRIBUTING.mdreviewed for alignmentRisk and Rollback
d7ff9c7Additional Notes
gh api repos/ndycode/codex-multi-auth/languagescurrently reportsTypeScript,JavaScript,HTML, andShell; this change is intended to stop the non-TypeScript helper files from counting toward the repo language bar after mergenpm testis still red on inheritedtest/proactive-refresh.test.tstimeouts (does not log when all accounts return no_refresh_token,refreshes multiple accounts in parallel,handles mixed success and failure results); the same three failures reproduce on a clean detachedorigin/mainworktreenote: greptile review for oc-chatgpt-multi-auth. cite files like
lib/foo.ts:123. confirm regression tests + windows concurrency/token redaction coverage.Greptile Summary
adds
.gitattributeslinguist overrides to exclude.js,.mjs,.sh, and.htmlfiles from github language stats, and locks the contract intest/documentation.test.tswith an exact-match governance assertion.the implementation is clean and the test handles windows
\ \line endings correctly viasplit(/\ ?\ /).Confidence Score: 5/5
safe to merge — metadata-only change with a single P2 observation about vendor .d.ts over-inclusion that doesn't affect correctness
both changed files are clean: the
.gitattributesachieves the stated goal and the governance test is correctly implemented with proper windows line-ending handling. the only finding is a P2 style note about vendor.d.tsfiles being force-included, which doesn't break anything..gitattributes— the*.ts linguist-detectableglob catches vendor declaration files; worth a follow-up if accurate attribution mattersImportant Files Changed
*.ts linguist-detectablewill also force-includevendor/*.d.tsdeclaration files which linguist would normally exclude as vendor code.gitattributes, asserting exact ordered content; handles windows line endings; correctly uses the existingread()helper scoped toprojectRootFlowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[GitHub Linguist scans repo] --> B{File extension?} B -->|*.ts| C[linguist-detectable = true\nall .ts files counted\nincl. vendor/*.d.ts + test/*.ts] B -->|*.js / *.mjs| D[-linguist-detectable\nexcluded from stats] B -->|*.sh| E[-linguist-detectable\nexcluded from stats] B -->|*.html| F[-linguist-detectable\nexcluded from stats] C --> G[Language bar: TypeScript only] D --> G E --> G F --> GPrompt To Fix All With AI
Reviews (2): Last reviewed commit: "Tighten the linguist override regression..." | Re-trigger Greptile