Skip to content

feat: linting edge-case gap remediation v5.1#28

Open
luandro wants to merge 65 commits intomainfrom
feat/linting-edge-case-gaps-v5.1
Open

feat: linting edge-case gap remediation v5.1#28
luandro wants to merge 65 commits intomainfrom
feat/linting-edge-case-gaps-v5.1

Conversation

@luandro
Copy link
Copy Markdown
Contributor

@luandro luandro commented Apr 13, 2026

Summary

Expand spreadsheet linting diagnostics to achieve full parity with the plugin's actual export/build behavior, closing high-impact gaps across Categories, Details, Metadata, Translations, and Icons sheets.

Context

The existing linting system (src/lint.ts) only covered basic validation — duplicate names, required fields, and simple type checking. Many conditions that cause build failures or silent data loss during config export were invisible to users until they hit "Generate Config". This implementation aligns lint diagnostics with the real workflows used by the export pipeline (payloadBuilder.ts) and strict validation (importService.ts).

Plan: plans/2026-04-12-linting-edge-case-gaps-v5.1.md

Changes

Phase 1 — Lint infrastructure and artifact lifecycle

  • Expanded managed color/font palette for consistent clearing
  • Added setLintNote() (standardized severity-based note writer), clearLintArtifacts() (combined clearing helper), inspectRawIds() (pre-cleanup whitespace detection)

Phase 2 — Categories and Details build-parity checks

  • Duplicate explicit/generated ID checks for column E (both sheets)
  • Slug-collision warnings when names produce identical fallback IDs
  • Full Applies column validation (observation/track coverage, missing column warning)
  • Empty-sheet error annotations
  • Manual ID hygiene (whitespace-only, non-slug-safe IDs)

Phase 3 — Field-reference, option, and type parity

  • Reuses normalizeFieldTokens() from builder for consistent delimiter handling (commas, semicolons, newlines, bullets, fullwidth punctuation)
  • Field resolution against both names AND explicit IDs from Details col E
  • parseCanonicalOptions() mirrors builder's parseOptions() (first-colon value:label split)
  • Ambiguous colon detection, type clarity advisories, ignored-options warnings on text/number fields

Phase 4 — Locale, Metadata, and translation parity

  • Primary language validation at Categories!A1 using validateLanguageName()
  • Metadata sheet lint for unsafe characters in name/version/primaryLanguage
  • Duplicate resolved locale detection in translation headers
  • Translation source overwrite warnings
  • Option-count delimiter alignment with builder (/[;,,、]/)

Phase 5 — Icon workflow and Icons sheet coverage

  • New lintIconsSheet() validates the Icons sheet (missing/duplicate IDs, unsupported formats)
  • Cross-sheet icon ID collision detection between Categories and Icons
  • Plain-text icon workflow-aware warnings (distinguishes "Generate Category Icons" from direct export)
  • Icon ID normalization warnings for .svg/.png suffixes

Phase 6 — Strict-validation advisory parity

  • Inline SVG size checks: warning >300KB, error >2MB
  • Per-locale translation payload size: error >1MB
  • Total entity count: error >10,000

Phase 7 — UI summary and regression coverage

  • Updated completion summary text describing all diagnostic categories
  • Clearing lifecycle fixes ensuring re-lint removes prior annotations
  • Parity test functions: testFieldTokenParity(), testCanonicalOptionParity(), testTranslationDelimiterParity()

Severity Matrix

Condition Severity Display
Build-blocking invalid value or collision Error #FFC7CE background + red font
Workflow-dependent or lossy behavior Warning #FFF2CC background + orange font
Advisory / quality guidance Advisory #FFFFCC background
Translation source/primary-column mismatch Critical Bright red + white text (preserved)

Testing

  1. Open the Google Spreadsheet and run Lint Sheets from the CoMapeo menu
  2. Verify the expanded summary alert describes all diagnostic categories
  3. Introduce test issues and verify correct severity annotations:
    • Duplicate a category name → slug collision warning on col A
    • Enter whitespace-only ID in col E → hygiene warning
    • Remove all "observation" from Applies column → error on header
    • Add plain text icon → workflow-aware warning
    • Add oversized inline SVG → size warning/error
  4. Fix issues and re-lint → verify all prior annotations are cleared
  5. Run parity tests from Apps Script editor: runPhase7ParityTests()

Files Changed

  • src/lint.ts — +1,791 / -232 lines (primary implementation, all 7 phases)

Greptile Summary

This PR expands the spreadsheet linting system across 7 phases to achieve build-parity with the export pipeline, closing high-impact diagnostic gaps in Categories, Details, Metadata, Translation, and Icons sheets. It also fixes a blank-type defaulting bug in payloadBuilder.ts (|| "text"|| "").

  • New lint infrastructure: setLintNote/appendLintNote/clearLintArtifacts helpers with severity-based styling and prefix-scoped clearing; all per-column clearing in lintSheet now includes clearRangeLintNoteLinesWithPrefix so stale notes are removed on re-lint.
  • Build-parity checks: duplicate effective-ID detection for col E, slug-collision warnings, Applies column coverage, field-reference resolution, option deduplication, locale/metadata validation, inline SVG size limits, entity-count guard, and a new lintIconsSheet() with cross-sheet collision detection.
  • Lifecycle fixes: note accumulation and overwrite bugs addressed via appendLintNote; Drive icon info cached at module level to avoid redundant API calls.

Confidence Score: 5/5

Safe to merge — all previously flagged blocking issues have been addressed and no new functional defects were found.

All prior review cycle issues are resolved: blank-type defaulting is fixed in both payloadBuilder.ts and the lint metrics builder, appendLintNote is used consistently to prevent note overwrites, checkDuplicateTranslationSlugs is now wired in, Drive icon MIME handling is correct, and validateSheetConsistency clears only data rows. The two remaining findings are a dead filter constant and a minor mismatch between the completion alert text and the actual version key behaviour — neither affects runtime correctness.

No files require special attention; src/lint.ts carries the bulk of the change but is well-structured and the critical call-ordering constraints are consistently applied.

Important Files Changed

Filename Overview
src/lint.ts Primary change (+1,791/-232 lines): adds 7 phases of build-parity lint checks, new helper infrastructure, and clears fixes from previous review cycles. A dead filter on LINT_WARNING_FONT_COLORS_WITHOUT_WHITE and a misleading version advisory note are minor issues.
src/builders/payloadBuilder.ts Two-line fix: blank type now defaults to "" (selectOne) instead of "text", and parseTokens gets a stronger type annotation — both safe changes.
src/importService.ts Widens mapFieldTypeToChar parameter from FieldType to string; safe refactor for flexibility.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[lintAllSheets] --> B[lintCategoriesSheet]
    A --> C[lintDetailsSheet]
    A --> D[lintIconsSheet]
    A --> E[checkCrossSheetIconCollisions]
    A --> F[checkInlineSvgSizes]
    A --> G[lintTranslationSheets]
    A --> H[collectStrictLintMetrics]
    A --> I[checkTranslationPayloadSizes]
    A --> J[lintMetadataSheet]
    A --> K[checkTotalEntityCounts]
    B --> B1[validatePrimaryLanguageInA1]
    B --> B2[lintSheet - cols A,B,C]
    B --> B3[validateCategoryIcons]
    B --> B4[checkDuplicateCategoryIds - col E]
    B --> B5[checkSlugCollisions - col A]
    B --> B6[validateAppliesColumn - col D]
    B --> B7[checkManualIdHygiene - col E]
    C --> C1[lintSheet - cols A-F]
    C --> C2[checkUnreferencedDetails]
    C --> C3[checkDuplicateDetailIds - col E]
    C --> C4[checkSlugCollisions - col A]
    C --> C5[checkManualIdHygiene - col E]
    G --> G1[validateTranslationHeaders]
    G --> G2[validateTranslationSheetConsistency]
    G --> G3[checkTranslationSourceOverwrites]
    G --> G4[checkDuplicateTranslationSlugs]
Loading
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
src/lint.ts:35-38
`LINT_WARNING_FONT_COLORS_WITHOUT_WHITE` always equals `LINT_WARNING_FONT_COLORS` because `"#FFFFFF"` is never in `["red", "orange", "#FF0000"]`. The filter is a no-op, so the variable name misleads future readers into thinking white text is being excluded from cleanup. The actual protection for white-on-red critical cells comes from the narrow `SOURCE_OVERWRITE_LINT_NOTE_PREFIX` scope and the fact that those cells carry no `[Lint]` note — not from this filter.

```suggestion
const LINT_WARNING_FONT_COLORS = ["red", "orange", "#FF0000"];
// Alias kept for clarity: white (#FFFFFF) is never in LINT_WARNING_FONT_COLORS,
// so source-overwrite cleanup already never clears white text. No filter needed.
const LINT_WARNING_FONT_COLORS_WITHOUT_WHITE = LINT_WARNING_FONT_COLORS;
```

### Issue 2 of 2
src/lint.ts:4145-4162
**`version` validation diverges from PR description**

The PR description, the summary in `lintAllSheets`, and the completion alert all state "Unsafe characters in name/version/primaryLanguage", implying `version` is checked for slashes/backslashes/ellipses like `name` and `primaryLanguage` are. The actual implementation gives an advisory when the value doesn't match the `yy.MM.dd` auto-generated pattern — not an unsafe-character error. The in-code comment explains why (builder always overwrites `version`), which is correct reasoning, but the mismatch with the documented behavior could confuse users who read the alert and expect their slash-containing version string to be flagged as an error. Consider updating the completion alert text to replace "name/version/primaryLanguage" with "name/primaryLanguage" and mention the `version` overwrite advisory separately.

Reviews (78): Last reviewed commit: "chore: stop tracking generated codesight..." | Re-trigger Greptile

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1a812ef58d

ℹ️ 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".

Comment thread src/lint.ts Outdated
Comment thread src/lint.ts Outdated
Comment thread src/lint.ts
@luandro
Copy link
Copy Markdown
Contributor Author

luandro commented Apr 13, 2026

@codex review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d6b4c12bb2

ℹ️ 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".

Comment thread src/lint.ts Outdated
Comment thread src/lint.ts Outdated
@luandro
Copy link
Copy Markdown
Contributor Author

luandro commented Apr 13, 2026

@codex review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 76aa2bc56c

ℹ️ 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".

Comment thread src/lint.ts Outdated
Comment thread src/lint.ts Outdated
Comment thread src/lint.ts Outdated
@luandro
Copy link
Copy Markdown
Contributor Author

luandro commented Apr 13, 2026

@codex review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 97ba536fb7

ℹ️ 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".

Comment thread src/lint.ts Outdated
Comment thread src/lint.ts Outdated
Comment thread src/lint.ts Outdated
Comment thread src/lint.ts Outdated
luandro added a commit that referenced this pull request Apr 14, 2026
Address 4 parity gaps flagged in PR #28 review (chatgpt-codex-connector):

1. Downgrade missing-track Applies finding to warning (was error).
   importService.runStrictBuildValidations() pushes no-track to warnings[],
   not errors[], so lint must not block users for a valid payload.

2. Align metadata unsafe-char pattern with containsUnsafeNameCharacters().
   Old UNSAFE_PATTERN (/[<>"'&\x00-\x1F]/) flagged apostrophes/ampersands
   as errors when the actual validator only rejects slashes/backslashes/ellipsis.
   New STRICT_UNSAFE_PATTERN = /[\\/]|\.\.\./

3. Reject non-SVG HTTP URLs in Icons sheet lint (P1).
   payloadBuilder.isSvgUrl() requires .svg in the URL; any other HTTP URL
   returns null from parseIconSource() and is silently dropped during build.
   Now emits an error for HTTP URLs without .svg, not a false-pass.

4. Use Categories icon-id column (col F) for cross-sheet collision checks.
   buildIconsFromSheet() uses the "icon id" column when present, overriding
   the category ID. Collision detection now reads col F and falls back to
   categoryId, avoiding false positives/negatives when icon id ≠ category id.
@luandro
Copy link
Copy Markdown
Contributor Author

luandro commented Apr 14, 2026

@codex review

chatgpt-codex-connector[bot]

This comment was marked as outdated.

luandro added a commit that referenced this pull request Apr 14, 2026
…check

checkTranslationSourceOverwrites() was calling setLintNote() which
unconditionally overwrites any existing note and severity styling.
validateTranslationSheetConsistency() runs before it and may write
critical (bright-red/white-text) annotations on the same cells.

Switch to appendLintNote() so source-overwrite warnings are appended
to — not replaced over — existing critical mismatch styling.

Closes Codex review finding (PR #28, line 2310).
@luandro
Copy link
Copy Markdown
Contributor Author

luandro commented Apr 14, 2026

@codex review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 05218c601b

ℹ️ 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".

Comment thread src/lint.ts Outdated
Comment thread src/lint.ts Outdated
luandro added a commit that referenced this pull request Apr 14, 2026
- checkForDuplicates: use clearLintArtifacts() to remove stale lint
  notes alongside backgrounds, preventing false-positive re-runs
- lintIconsSheet: restrict Drive URL check to /file/d/ format only,
  matching payloadBuilder.isGoogleDriveLink() and preventing open?id=
  links from passing lint while being silently dropped during build
- checkIconIdNormalization: read 6 cols and use col-F icon-id override
  (mirroring buildIconsFromSheet), add parseIconSource guard to skip
  rows the builder ignores, add Math.max column guard to avoid range
  errors on narrow sheets
luandro added a commit that referenced this pull request Apr 14, 2026
Address 4 parity gaps flagged in PR #28 review (chatgpt-codex-connector):

1. Downgrade missing-track Applies finding to warning (was error).
   importService.runStrictBuildValidations() pushes no-track to warnings[],
   not errors[], so lint must not block users for a valid payload.

2. Align metadata unsafe-char pattern with containsUnsafeNameCharacters().
   Old UNSAFE_PATTERN (/[<>"'&\x00-\x1F]/) flagged apostrophes/ampersands
   as errors when the actual validator only rejects slashes/backslashes/ellipsis.
   New STRICT_UNSAFE_PATTERN = /[\\/]|\.\.\./

3. Reject non-SVG HTTP URLs in Icons sheet lint (P1).
   payloadBuilder.isSvgUrl() requires .svg in the URL; any other HTTP URL
   returns null from parseIconSource() and is silently dropped during build.
   Now emits an error for HTTP URLs without .svg, not a false-pass.

4. Use Categories icon-id column (col F) for cross-sheet collision checks.
   buildIconsFromSheet() uses the "icon id" column when present, overriding
   the category ID. Collision detection now reads col F and falls back to
   categoryId, avoiding false positives/negatives when icon id ≠ category id.
luandro added a commit that referenced this pull request Apr 14, 2026
…check

checkTranslationSourceOverwrites() was calling setLintNote() which
unconditionally overwrites any existing note and severity styling.
validateTranslationSheetConsistency() runs before it and may write
critical (bright-red/white-text) annotations on the same cells.

Switch to appendLintNote() so source-overwrite warnings are appended
to — not replaced over — existing critical mismatch styling.

Closes Codex review finding (PR #28, line 2310).
luandro added a commit that referenced this pull request Apr 14, 2026
- checkForDuplicates: use clearLintArtifacts() to remove stale lint
  notes alongside backgrounds, preventing false-positive re-runs
- lintIconsSheet: restrict Drive URL check to /file/d/ format only,
  matching payloadBuilder.isGoogleDriveLink() and preventing open?id=
  links from passing lint while being silently dropped during build
- checkIconIdNormalization: read 6 cols and use col-F icon-id override
  (mirroring buildIconsFromSheet), add parseIconSource guard to skip
  rows the builder ignores, add Math.max column guard to avoid range
  errors on narrow sheets
@luandro luandro force-pushed the feat/linting-edge-case-gaps-v5.1 branch from eb6a85c to a2eed87 Compare April 14, 2026 13:26
@luandro
Copy link
Copy Markdown
Contributor Author

luandro commented Apr 14, 2026

@codex review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a2eed871d1

ℹ️ 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".

Comment thread src/lint.ts Outdated
Comment thread src/lint.ts Outdated
Comment thread src/lint.ts Outdated
@greptile-apps

This comment was marked as outdated.

Comment thread src/lint.ts Outdated
Comment thread src/lint.ts Outdated
Comment thread src/lint.ts
luandro added a commit that referenced this pull request Apr 15, 2026
Address 4 parity gaps flagged in PR #28 review (chatgpt-codex-connector):

1. Downgrade missing-track Applies finding to warning (was error).
   importService.runStrictBuildValidations() pushes no-track to warnings[],
   not errors[], so lint must not block users for a valid payload.

2. Align metadata unsafe-char pattern with containsUnsafeNameCharacters().
   Old UNSAFE_PATTERN (/[<>"'&\x00-\x1F]/) flagged apostrophes/ampersands
   as errors when the actual validator only rejects slashes/backslashes/ellipsis.
   New STRICT_UNSAFE_PATTERN = /[\\/]|\.\.\./

3. Reject non-SVG HTTP URLs in Icons sheet lint (P1).
   payloadBuilder.isSvgUrl() requires .svg in the URL; any other HTTP URL
   returns null from parseIconSource() and is silently dropped during build.
   Now emits an error for HTTP URLs without .svg, not a false-pass.

4. Use Categories icon-id column (col F) for cross-sheet collision checks.
   buildIconsFromSheet() uses the "icon id" column when present, overriding
   the category ID. Collision detection now reads col F and falls back to
   categoryId, avoiding false positives/negatives when icon id ≠ category id.
luandro added a commit that referenced this pull request Apr 15, 2026
…check

checkTranslationSourceOverwrites() was calling setLintNote() which
unconditionally overwrites any existing note and severity styling.
validateTranslationSheetConsistency() runs before it and may write
critical (bright-red/white-text) annotations on the same cells.

Switch to appendLintNote() so source-overwrite warnings are appended
to — not replaced over — existing critical mismatch styling.

Closes Codex review finding (PR #28, line 2310).
luandro added a commit that referenced this pull request Apr 15, 2026
- checkForDuplicates: use clearLintArtifacts() to remove stale lint
  notes alongside backgrounds, preventing false-positive re-runs
- lintIconsSheet: restrict Drive URL check to /file/d/ format only,
  matching payloadBuilder.isGoogleDriveLink() and preventing open?id=
  links from passing lint while being silently dropped during build
- checkIconIdNormalization: read 6 cols and use col-F icon-id override
  (mirroring buildIconsFromSheet), add parseIconSource guard to skip
  rows the builder ignores, add Math.max column guard to avoid range
  errors on narrow sheets
@luandro luandro force-pushed the feat/linting-edge-case-gaps-v5.1 branch from e9c5aa5 to a368a7d Compare April 15, 2026 10:28
luandro added a commit that referenced this pull request Apr 15, 2026
Address 4 parity gaps flagged in PR #28 review (chatgpt-codex-connector):

1. Downgrade missing-track Applies finding to warning (was error).
   importService.runStrictBuildValidations() pushes no-track to warnings[],
   not errors[], so lint must not block users for a valid payload.

2. Align metadata unsafe-char pattern with containsUnsafeNameCharacters().
   Old UNSAFE_PATTERN (/[<>"'&\x00-\x1F]/) flagged apostrophes/ampersands
   as errors when the actual validator only rejects slashes/backslashes/ellipsis.
   New STRICT_UNSAFE_PATTERN = /[\\/]|\.\.\./

3. Reject non-SVG HTTP URLs in Icons sheet lint (P1).
   payloadBuilder.isSvgUrl() requires .svg in the URL; any other HTTP URL
   returns null from parseIconSource() and is silently dropped during build.
   Now emits an error for HTTP URLs without .svg, not a false-pass.

4. Use Categories icon-id column (col F) for cross-sheet collision checks.
   buildIconsFromSheet() uses the "icon id" column when present, overriding
   the category ID. Collision detection now reads col F and falls back to
   categoryId, avoiding false positives/negatives when icon id ≠ category id.
luandro added a commit that referenced this pull request Apr 15, 2026
…check

checkTranslationSourceOverwrites() was calling setLintNote() which
unconditionally overwrites any existing note and severity styling.
validateTranslationSheetConsistency() runs before it and may write
critical (bright-red/white-text) annotations on the same cells.

Switch to appendLintNote() so source-overwrite warnings are appended
to — not replaced over — existing critical mismatch styling.

Closes Codex review finding (PR #28, line 2310).
luandro added a commit that referenced this pull request Apr 15, 2026
- checkForDuplicates: use clearLintArtifacts() to remove stale lint
  notes alongside backgrounds, preventing false-positive re-runs
- lintIconsSheet: restrict Drive URL check to /file/d/ format only,
  matching payloadBuilder.isGoogleDriveLink() and preventing open?id=
  links from passing lint while being silently dropped during build
- checkIconIdNormalization: read 6 cols and use col-F icon-id override
  (mirroring buildIconsFromSheet), add parseIconSource guard to skip
  rows the builder ignores, add Math.max column guard to avoid range
  errors on narrow sheets
@luandro luandro force-pushed the feat/linting-edge-case-gaps-v5.1 branch from 3d6c505 to 6d5bf69 Compare April 15, 2026 10:36
Comment thread src/lint.ts
Comment thread src/lint.ts
Comment thread src/lint.ts
- lintIconsSheet: skip fully blank rows (builder ignores them too)
- Invalid type: replace bare setInvalidCellBackground with setLintNote
  for consistent hover diagnostics
- lintMetadataSheet: remove redundant seenKeys.add in duplicate branch
@luandro
Copy link
Copy Markdown
Contributor Author

luandro commented Apr 24, 2026

Twenty-Ninth-Round Review Analysis (commit 1b59c92)

Analyzed the 14 unresolved review threads. 1 actionable, 13 by-design/recurring.

Fixes Applied (3 findings)

# File Issue Fix
1 src/lint.ts:2514-2516 lintIconsSheet() treats fully blank spacer rows as errors, unlike builder which skips them Added if (!iconId && !iconSource) continue; before the missing-ID check, matching buildIconsFromSheet() behavior
2 src/lint.ts:2303-2313 Invalid type uses bare setInvalidCellBackground with no hover note — users see red cell but no explanation Replaced with setLintNote() for consistent hover diagnostics
3 src/lint.ts:4099 Redundant seenKeys.add(effectiveKey) inside the duplicate branch — key already in set Removed the no-op add

By Design / Not in Scope (11 findings)

# Thread Finding Rationale
1-5 PRRT_kwDONCSAJs58o8oa, PRRT_kwDONCSAJs58qjR4, PRRT_kwDONCSAJs58qv1-, PRRT_kwDONCSAJs58q5gt, PRRT_kwDONCSAJs58rs76 Hard-coded columns migrateSpreadsheetFormat() enforces canonical layout
6 PRRT_kwDONCSAJs58rk07 Hard-coded columns in icon checks Same as #1-5
7 PRRT_kwDONCSAJs58rk00 Base-code primary drops variants Correct: user chose base code, all variants excluded
8 PRRT_kwDONCSAJs58rs8V Same as #7 Same rationale
9 PRRT_kwDONCSAJs58rs73 Reader SVG sanitization Reader reads archives as-is; sanitization happens at write time via parseSvg()
10 PRRT_kwDONCSAJs59QARl tsconfig excludes test files Deliberate: GAS tests share global scope; globals.d.ts provides type declarations
11 PRRT_kwDONCSAJs59QAS4 primaryLanguage duplicate row messaging Message correctly describes buildLocales() builder behavior, not getPrimaryLanguage() accessor

All fixes pass biome lint and tsc --noEmit. 0 unresolved threads remaining.

@luandro
Copy link
Copy Markdown
Contributor Author

luandro commented Apr 24, 2026

@codex review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1b59c9258b

ℹ️ 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".

Comment thread src/lint.ts
Comment thread src/lint.ts
…lationSlugs

The function filtered out blank cells before computing row indices, so
i+2 no longer mapped to the correct sheet row when blanks were present.
This caused lint warnings to be written to wrong cells.

Fix: iterate raw values in-place with continue (matching the pattern in
checkTranslationSourceOverwrites) so the array index always corresponds
to the physical sheet row.

Thread: PRRT_kwDONCSAJs59S7Gl
@luandro
Copy link
Copy Markdown
Contributor Author

luandro commented Apr 24, 2026

Thirtieth-Round Review Analysis (commit 7b63e39)

Analyzed the 11 unresolved review threads. 1 actionable, 10 by-design/recurring.

Fixes Applied (1 finding)

# File Issue Fix
1 src/lint.ts:1400-1418 checkDuplicateTranslationSlugs() filtered blank cells before computing row indices, so i+2 no longer mapped to the correct sheet row when blanks were present Replaced .filter() with in-place iteration using continue, matching the pattern in checkTranslationSourceOverwrites()

Already Aligned (1 finding)

# Thread Finding Status
1 PRRT_kwDONCSAJs59S7Gh Option-count delimiter parsing No fix needed: lint's /[;,,、]/ at lint.ts:3178 is already aligned with builder's splitTranslatedOptions() at payloadBuilder.ts:1049. processTranslations() is dead code with zero callers.

By Design / Not in Scope (9 findings)

# Thread Finding Rationale
1-4 PRRT_kwDONCSAJs58o8oa, PRRT_kwDONCSAJs58qjR4, PRRT_kwDONCSAJs58qv1-, PRRT_kwDONCSAJs58q5gt Hard-coded columns migrateSpreadsheetFormat() enforces canonical layout
5 PRRT_kwDONCSAJs58rk07 Hard-coded columns in icon checks Same as #1-4
6 PRRT_kwDONCSAJs58rs76 Hard-coded columns in ID checks Same as #1-4
7 PRRT_kwDONCSAJs58rk00 Base-code primary drops variants Correct: user chose base code, all variants excluded
8 PRRT_kwDONCSAJs58rs8V Same as #7 Same rationale
9 PRRT_kwDONCSAJs58rs73 Reader SVG sanitization Reader reads archives as-is; sanitization happens at write time via parseSvg()

All fixes pass biome lint and tsc --noEmit. 0 unresolved threads remaining.

@luandro
Copy link
Copy Markdown
Contributor Author

luandro commented Apr 24, 2026

@codex review

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7b63e395d2

ℹ️ 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".

Comment thread src/spreadsheetData.ts Outdated
Comment thread src/lint.ts
Copy link
Copy Markdown

@capy-ai capy-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added 2 comments

Comment thread src/lint.ts
Comment thread src/importCategory/fileExtractor.ts
Copy link
Copy Markdown

@capy-ai capy-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added 1 comment

Comment thread src/lint.ts
luandro added 2 commits April 25, 2026 08:36
The pre-flight clearing in validateAppliesColumn() covered two note
prefixes on the body range but omitted the third prefix written per
data row: 'Applies value contains semicolons or other non-comma
delimiters'. When the Applies column was present on run 1 (delimiter
warnings written to data cells) and renamed/removed on run 2, those
notes persisted indefinitely because the function returned early
before reaching clearLintArtifacts(appliesRange).

Added the missing clearRangeLintNoteLinesWithPrefix call.
…sets

processTranslations() initialized messages only from Category Translations
languages. When other sheets (Detail Label, etc.) had additional languages,
writes to messages[lang] would crash with undefined.

Now unions all languages across all sheets before initializing messages.

Also fixes locale variant lowercasing in translationHeaderResolver: unknown
locale tags like pt-BR were returned as pt-br. Now preserves BCP 47 casing.
Copy link
Copy Markdown

@capy-ai capy-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added 3 comments

Comment thread src/validation.ts Outdated
Comment thread package/bin/comapeocat-build.mjs
Comment thread src/importCategory/fileExtractor.ts
Comment claimed the variable tracked non-empty values regardless of
validity, but code only sets it when isValid is true. Renamed to
seenValidPrimaryLanguage and updated comment to match actual behavior.
Copy link
Copy Markdown

@capy-ai capy-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added 1 comment

Comment thread package/src/reader.js Outdated
Copy link
Copy Markdown

@capy-ai capy-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added 3 comments

Comment thread src/lint.ts Outdated
Comment thread src/lint.ts
Comment thread src/translationHeaderResolver.ts
luandro added 6 commits May 6, 2026 12:09
The GAS-side locale regex in normalizePrimaryLanguageLocaleCode rejected
valid UN M.49 numeric region subtags like es-419, while the package's
validateBcp47 (via bcp-47-normalize + unM49 checks) accepted them. This
created a lint parity gap where the spreadsheet linter would flag a locale
that the builder would successfully build.

Update the regex from /^[a-z]{2,3}(-[a-z]{2,3})?$/i to also accept 3-digit
numeric subtags: /^[a-z]{2,3}(-[a-z]{2,3}|-\d{3})?$/i.

This aligns the GAS linter with the package builder's BCP-47 validation.

Closes capy-ai thread PRRT_kwDONCSAJs59j6yC.
…LanguageInA1

validatePrimaryLanguageInA1() already clears A1 artifacts at line 2003.
The duplicate clear at the call site was redundant and fragile — if a
future lint phase writes to A1 between the two calls, the inner clear
would wipe it.
@capy-ai
Copy link
Copy Markdown

capy-ai Bot commented May 6, 2026

Capy auto-review is paused for this organization because the monthly auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant