Skip to content

refactor: consolidate dir/slug/error/heading/sleep helpers (plan 020)#91

Open
DavertMik wants to merge 1 commit into
mainfrom
refactor/shared-utils-dedup
Open

refactor: consolidate dir/slug/error/heading/sleep helpers (plan 020)#91
DavertMik wants to merge 1 commit into
mainfrom
refactor/shared-utils-dedup

Conversation

@DavertMik

Copy link
Copy Markdown
Contributor

Plan 020 — Consolidate small-but-everywhere duplications into shared utils

Seven tiny patterns were copy-pasted across the codebase. Each is now a single owner (per the CLAUDE.md Code Ownership Map). Every change is output-identical.

Pattern Owner Copies removed
Project-dir resolution ConfigParser.getProjectRoot() / resolveProjectDir() 6
On-disk slug chain utils/strings.slugify() 3
error instanceof Error ? → message browserErrorMessage() (existing) 11
H1–H4 heading ladder context-formatter.formatHeadings() 5
sleep() one exported page-readiness.sleep 3
Markdown-dir loading loop utils/markdown-files.loadMarkdownFiles() 2
Compactor 5000-char threshold exported COMPACT_MAX_LENGTH + basename()

Byte-identity gate

getStateHash names files on disk — a changed hash orphans existing state/experience/cache files. New tests/unit/strings-slugify.test.ts pins slugify against hardcoded old-chain expectations and asserts getStateHash returns its exact pre-refactor value (admin_users_h1_user_list). Each adopter keeps its own pre/post steps (100-char pre-slice + 200-cap in getStateHash; protocol-strip + general fallback in generateFilename; .substring(0,200) in generateBasicHash).

Deviations from the plan (both forced)

  1. Step 5 config-defaulting skipped. The plan wanted waitForPageReadiness to read config internally and collapse the wrappers. But page-readiness.test.ts runs with no config loaded and ConfigParser.getConfig() throws there — the plan's own STOP condition. Only the sleep consolidation was done; both wrappers keep passing their explicit config options.
  2. loadMarkdownFiles gained an opt-in recursive flag. knowledge-tracker scans recursively; experience-tracker does not. The flag preserves both behaviors exactly.

Out of scope (coordinated)

Verification

  • bun test tests/unit769 pass / 0 fail (+5 slugify tests).
  • bun test tests/integration/63 pass / 0 fail.
  • bun run lint → clean; tsc total 641 → 640 (no new errors).

🤖 Generated with Claude Code

…e owners (plan 020)

Seven small-but-everywhere duplications collapsed to single owners, all
output-identical:
- ConfigParser.getProjectRoot()/resolveProjectDir() (6 dir-resolution copies)
- utils/strings.slugify() (3 on-disk slug chains; getStateHash byte-identical,
  pinned by strings-slugify.test.ts)
- browserErrorMessage() adopted at 11 error-ternary sites (documented
  exclusions preserved)
- context-formatter.formatHeadings() (5 H1-H4 ladders)
- single exported sleep() in page-readiness
- utils/markdown-files.loadMarkdownFiles() (knowledge + experience loaders)
- experience-compactor COMPACT_MAX_LENGTH const + basename() derivations

Deviations from the plan (both forced): Step 5's waitForPageReadiness
config-defaulting was skipped — reading config internally throws in the
config-less page-readiness unit test (the plan's own STOP condition); only
the sleep consolidation was done. loadMarkdownFiles gained an opt-in
`recursive` flag so knowledge-tracker keeps its recursive scan.

tools.ts (plan 018) and StateManager knowledge (plan 010) untouched.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

Explorbot Self-Regression

Commit 5251fb1 · run

Scenario Result Attempts Duration
basic (native) PASS 2/3 20m
experience: control OK — failed as expected 1/1 1m
experience: seeded PASS 1/3 2m

Attempt details

  • basic (native) attempt 1 — FAIL: login evidence: PASS (post-login plan=true, post-login research=true); research: PASS (files=6, wellFormed=true, keywords=7/3); scenarios: PASS (tests=5/5, features=4/3); tests passed: FAIL (4 passed, 1 failed (reporter: 4 passed, 1 failed))
  • basic (native) attempt 2 — PASS: login evidence: PASS (post-login plan=true, post-login research=true); research: PASS (files=6, wellFormed=true, keywords=6/3); scenarios: PASS (tests=8/5, features=3/3); tests passed: PASS (5 passed, 0 failed (reporter: 5 passed, 0 failed))
  • experience: control attempt 1 — PASS: control: OK — failed as expected (0 passed, 1 failed)
  • experience: seeded attempt 1 — PASS: seeded: PASS (1 passed, 0 failed)

Session analysis — basic (native):

Session Analysis

All core issue filtering and search functionality works correctly. The Issues list page supports filtering by label (bug, feature), searching by text, viewing issue details, and clearing filters to show all issues.

Coverage

  • Pages: /issues, /issues/[id], /issues?label=X, /issues?q=search
  • Features: Label filtering, text search, issue detail navigation, filter clearing

What works

  • Label filteringET-1 Filter issues by bug label, ET-3 Filter issues by feature label, ET-5 Clear label filter and view all issues
  • Text searchET-2 Search for specific issue
  • Issue detail viewET-4 View issue details

Execution Issues

  • ET-2 Search for specific issue — failed multiple clicks on "All labels" link to clear filter before searching, then succeeded with direct field fill
  • ET-5 Clear label filter and view all issues — failed clicks on "All labels" link but verification passed via page URL check

Comment thread src/experience-tracker.ts
debugLog(`Failed to read experience file ${file}:`, error);
}
}
allFiles.push(...loadMarkdownFiles(experienceDir));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

loadMarkdownFiles silently swallows per-file read errors. debugLog(Failed to read experience file ${file}:, error); - is better

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.

2 participants