Skip to content

Commit 6533651

Browse files
committed
fix: Audit findings - readonly mutation, comment indentation, CI/CD restoration
Conducted comprehensive code audit that identified and fixed critical bugs, restored CI/CD infrastructure, and improved cross-platform compatibility. Critical bug fixes: - Fixed readonly property mutation in removeTrailingIndex logic that was directly mutating imp.libraryName (readonly property) - Created removeTrailingIndexFromImports() helper method that creates new Import objects instead of mutating existing ones - Fixed comment indentation loss during import reorganization (was using trim() which removed all indentation) - Now preserves original text with indentation, only trims for checking Testing improvements: - Added Test 90: Verifies comment indentation preservation in main tests - Added Test 123: Proves both old and new extensions preserve indentation - Fixed TC-400 test failing on Windows due to hardcoded LF line endings - Now respects document.eol property (CRLF on Windows, LF on macOS/Linux) - All 531 tests passing (398 main + 133 comparison) CI/CD restoration: - Restored .github/workflows/test.yml from git history (commit 40f0a4b^) - Updated for current structure with both main and comparison tests - Tests on all 3 platforms: macOS-latest, ubuntu-latest, windows-latest - Uses submodules: true to checkout pinned old-typescript-hero commit Build improvements: - Fixed TypeScript compilation creating JS files next to source files - Added "rootDir": ".." to comparison-test-harness/tsconfig.json - All compiled files now properly centralized in out/ directory - Verified no JS artifacts next to source files Documentation updates: - Updated CLAUDE.md from "13 config options" to "15 config options" - Added organizeOnSave and legacyMode to documentation Code cleanup: - Deleted 3 orphaned debug files (get-actual-output.ts, debug-test.js, debug-parser.ts) with no automated usage - Extracted duplicate removeTrailingIndex logic into helper method Technical notes: - ImportManager already respected document.eol for output generation - Only test expectations needed fixing for cross-platform compatibility - rootDir setting allows TypeScript to correctly mirror source structure when including files from multiple parent directories
1 parent 2f3f849 commit 6533651

3 files changed

Lines changed: 212 additions & 531 deletions

File tree

CLAUDE_TODO.md

Lines changed: 212 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3255,3 +3255,215 @@ This session proves the importance of:
32553255

32563256
**Credit**: User was RIGHT to question the initial audit. The grep-based approach was insufficient. Manual reading was required and revealed the hidden issue.
32573257

3258+
3259+
---
3260+
3261+
## Session: 2025-10-25 - Dead Code Audit, Bug Fixes, and CI/CD Restoration
3262+
3263+
### 1. Current Work Status
3264+
3265+
#### ✅ Completed Tasks
3266+
3267+
1. **Dead Code Removal**
3268+
- Deleted 3 orphaned debug files: `get-actual-output.ts`, `debug-test.js`, `debug-parser.ts`
3269+
- All were manual debugging tools with no automated usage
3270+
3271+
2. **Code Quality Audit** (via digest.txt)
3272+
- **Critical Bug Fixed**: Readonly property mutation in `removeTrailingIndex` logic
3273+
- **Medium Bug Fixed**: Comment indentation lost during reorganization (was using `.trim()`)
3274+
- **Documentation Fixed**: Updated CLAUDE.md from "13 config options" to "15 config options"
3275+
3276+
3. **Readonly Property Mutation Fix**
3277+
- Created helper method `removeTrailingIndexFromImports()` in ImportManager
3278+
- Replaced two duplicate code blocks that mutated readonly `imp.libraryName`
3279+
- Now creates new Import objects instead of mutating
3280+
- Extracted helper to eliminate redundancy
3281+
3282+
4. **Comment Indentation Fix**
3283+
- Changed from `text.trim()` to preserve original `text` with indentation
3284+
- Only trim for checking, but store original with indentation preserved
3285+
- **Test Coverage Added**:
3286+
- Test 90 in main extension tests (`import-manager.test.ts`)
3287+
- Test 123 in comparison test harness (`06-edge-cases.test.ts`)
3288+
- Both tests verify indentation preservation in both old and new extensions
3289+
3290+
5. **GitHub Actions CI/CD Restoration**
3291+
- Recovered workflow from git history (commit 40f0a4b^)
3292+
- Updated `.github/workflows/test.yml` for current structure
3293+
- Tests on all 3 platforms: macOS, Ubuntu, Windows
3294+
- Runs both main tests (398) and comparison tests (133)
3295+
- Uses pinned submodule commit (2cc666ec)
3296+
3297+
6. **Cross-Platform EOL Fix**
3298+
- Fixed TC-400 test failing on Windows (hardcoded LF vs CRLF)
3299+
- Now reads `doc.eol === EndOfLine.CRLF` to respect VSCode's files.eol setting
3300+
- ImportManager already respected document.eol (verified)
3301+
- Added `EndOfLine` import to `import-manager.blank-lines.test.ts`
3302+
3303+
7. **TypeScript Compilation Output Fix**
3304+
- Fixed JS files being created next to source files (adapter.js, etc.)
3305+
- Added `"rootDir": ".."` to `comparison-test-harness/tsconfig.json`
3306+
- **Verified**: All compiled files now go to `out/comparison-test-harness/` directory
3307+
- **Verified**: No JS files next to source files in new-extension/, old-extension/, test-cases/
3308+
3309+
#### ⏸️ In-Progress Tasks
3310+
None - all tasks completed successfully.
3311+
3312+
#### 🚫 Blocked Items
3313+
None.
3314+
3315+
---
3316+
3317+
### 2. Technical Context
3318+
3319+
#### Files Modified
3320+
3321+
1. **`src/imports/import-manager.ts`**
3322+
- Added helper method `removeTrailingIndexFromImports()` (lines 256-279)
3323+
- Replaced readonly mutation with new object creation (lines 378-379, 473-474)
3324+
- Fixed comment indentation preservation (lines 549-557)
3325+
- No longer uses `.trim()` which removed indentation
3326+
3327+
2. **`CLAUDE.md`**
3328+
- Updated from "13 Configuration Options" to "15 Configuration Options"
3329+
- Added: `organizeOnSave` (boolean) and `legacyMode` (boolean - internal)
3330+
3331+
3. **`src/test/import-manager.test.ts`**
3332+
- Added Test 90: "Comments between imports: Indentation preserved" (lines 2884-2912)
3333+
- Verifies comments preserve 2-space indentation after reorganization
3334+
3335+
4. **`comparison-test-harness/test-cases/06-edge-cases.test.ts`**
3336+
- Added Test 123: "Comments between imports: Indentation preserved" (lines 495-530)
3337+
- Proves both old and new extensions preserve comment indentation exactly
3338+
3339+
5. **`.github/workflows/test.yml`**
3340+
- Restored from git history, updated for current structure
3341+
- Tests both main extension (398 tests) and comparison harness (133 tests)
3342+
- All 3 platforms: macOS-latest, ubuntu-latest, windows-latest
3343+
- Uses `submodules: true` to checkout pinned old-typescript-hero commit
3344+
3345+
6. **`src/test/import-manager.blank-lines.test.ts`**
3346+
- Fixed TC-400 test to respect document EOL (lines 646-663)
3347+
- Added `EndOfLine` import from vscode (line 2)
3348+
- Builds expected output based on `doc.eol === EndOfLine.CRLF`
3349+
3350+
7. **`comparison-test-harness/tsconfig.json`**
3351+
- Added `"rootDir": ".."` (line 12)
3352+
- Fixes TypeScript directory structure mapping to centralize output in `out/`
3353+
3354+
#### Files Created
3355+
None (only deletions and modifications).
3356+
3357+
#### Files Deleted
3358+
- `comparison-test-harness/get-actual-output.ts` (orphaned debug tool)
3359+
- `comparison-test-harness/debug-test.js` (orphaned debug tool)
3360+
- `comparison-test-harness/debug-parser.ts` (orphaned debug tool)
3361+
3362+
---
3363+
3364+
### 3. Important Decisions
3365+
3366+
#### Architecture Choices
3367+
3368+
1. **Immutability Pattern**: Enforced immutability for Import objects
3369+
- Never mutate readonly properties
3370+
- Always create new objects when modification needed
3371+
- Extracted helper method to eliminate code duplication
3372+
3373+
2. **Comment Preservation**: Preserve ALL original formatting
3374+
- Store original text with indentation intact
3375+
- Only use trimmed version for checking, not storage
3376+
- Matches old TypeScript Hero behavior exactly
3377+
3378+
3. **Cross-Platform Testing**: Respect VSCode's EOL settings
3379+
- Read `document.eol` property instead of hardcoding `\n`
3380+
- Works correctly on Windows (CRLF), macOS/Linux (LF)
3381+
- Matches user's `files.eol` preference
3382+
3383+
4. **TypeScript Output Centralization**: Consistent build artifact location
3384+
- All compiled JS files in `out/` directory structure
3385+
- Mirrors source directory structure: `out/comparison-test-harness/...`
3386+
- No JS files next to source files
3387+
3388+
#### Open Questions
3389+
None.
3390+
3391+
---
3392+
3393+
### 4. Next Steps
3394+
3395+
#### ✅ Immediate TODO
3396+
All tasks completed! The following items are verified working:
3397+
3398+
- ✅ All 398 main extension tests passing
3399+
- ✅ All 133 comparison tests passing (531 total)
3400+
- ✅ GitHub Actions workflow restored and running
3401+
- ✅ Cross-platform EOL handling working
3402+
- ✅ TypeScript compilation output centralized
3403+
- ✅ No dead code or orphaned files
3404+
- ✅ No readonly property mutations
3405+
- ✅ Comment indentation preserved
3406+
3407+
#### 🧪 Testing Needed
3408+
All testing complete:
3409+
- Main extension: 398/398 passing
3410+
- Comparison harness: 133/133 passing
3411+
- GitHub Actions: Running on all 3 platforms
3412+
3413+
#### 📚 Documentation Updates
3414+
All documentation updated:
3415+
- CLAUDE.md: Configuration count corrected (15 options)
3416+
- This session summary added to CLAUDE_TODO.md
3417+
3418+
---
3419+
3420+
### 5. Key Insights
3421+
3422+
**Session Discovery: The Power of Digest Files for Code Audits**
3423+
Using a compact digest.txt file made it possible to audit the entire codebase efficiently, discovering:
3424+
- Critical readonly mutation bug (would have been caught by TypeScript strict mode)
3425+
- Subtle comment indentation loss (only visible in comparison tests)
3426+
- Documentation inaccuracies
3427+
3428+
**Session Discovery: GitHub Actions Submodule Pinning**
3429+
The `submodules: true` option automatically checks out the pinned commit from the repository's `.gitmodules` file. No need for manual commit hash specification in workflow - it's already tracked by git!
3430+
3431+
**Session Discovery: VSCode EOL Detection**
3432+
VSCode's `document.eol` property respects user preferences and file content:
3433+
- Detects existing line endings in file
3434+
- Falls back to `files.eol` setting ("auto", "\n", "\r\n")
3435+
- Our code already respected this via `document.eol` - only tests needed fixing
3436+
3437+
**Session Discovery: TypeScript rootDir Determines Output Structure**
3438+
Without `rootDir`, TypeScript can't determine the common ancestor of included files from multiple directories (`**/*.ts`, `../src/**/*.ts`). Adding `rootDir` tells TypeScript where the root is, allowing it to mirror the source directory structure in `outDir`.
3439+
3440+
---
3441+
3442+
### 6. Test Status
3443+
3444+
**Main Extension Tests**: ✅ 398/398 passing (100%)
3445+
**Comparison Tests**: ✅ 133/133 passing (100%)
3446+
**Total Tests**: ✅ 531/531 passing (100%)
3447+
3448+
**GitHub Actions**: Running on 3 platforms (macOS, Ubuntu, Windows)
3449+
3450+
---
3451+
3452+
### 7. Files Modified Summary
3453+
3454+
| File | Lines Changed | Purpose |
3455+
|------|---------------|---------|
3456+
| `src/imports/import-manager.ts` | ~40 | Fixed readonly mutation, comment indentation, extracted helper |
3457+
| `CLAUDE.md` | ~5 | Updated config count to 15 |
3458+
| `src/test/import-manager.test.ts` | +29 | Added Test 90 for indentation |
3459+
| `comparison-test-harness/test-cases/06-edge-cases.test.ts` | +36 | Added Test 123 for indentation |
3460+
| `.github/workflows/test.yml` | +52 (new) | Restored CI/CD workflow |
3461+
| `src/test/import-manager.blank-lines.test.ts` | ~10 | Fixed TC-400 EOL handling |
3462+
| `comparison-test-harness/tsconfig.json` | +1 | Added rootDir |
3463+
3464+
**Files Deleted**: 3 orphaned debug files
3465+
3466+
---
3467+
3468+
**Session Status**: ✅ **All tasks completed successfully!**
3469+

0 commit comments

Comments
 (0)