Document security patterns, timing constants, and pre-submission checklist

[bensonwong]

Summary

This PR adds comprehensive documentation for critical development patterns and pre-submission requirements:

1. Security Patterns (CLAUDE.md): Documents dedicated security utilities in src/utils/ with examples for URL domain matching, prototype pollution prevention, ReDoS prevention, log injection prevention, and image source validation. Emphasizes never using ad-hoc patterns for these security-sensitive operations.

2. Timing Constants (CLAUDE.md): Documents the carefully calibrated popover timing values (HOVER_CLOSE_DELAY_MS, REPOSITION_GRACE_PERIOD_MS, SPINNER_TIMEOUT_MS, TOUCH_CLICK_DEBOUNCE_MS) with their purposes and test coverage. Clarifies these are deliberate and well-tested, not race conditions.

3. Type Safety Guidelines (CLAUDE.md): Adds rules for discriminated unions, unsafe casts, and export verification to prevent common type-related bugs.

4. Pre-Submission Checklist (AGENTS.md): Documents the required checks before opening a PR, with emphasis on running npm run check:fix first (the most common CI failure). Includes bundle size limits and export verification.

5. Testing Rules (AGENTS.md): Clarifies that tests must validate existing implemented behavior, documents current test coverage (28 unit test files, 9 Playwright specs), and provides guidance on when to add tests.

6. PR Description Guidelines (AGENTS.md): Requires PR descriptions to match the actual diff and calls out security changes and breaking changes explicitly.

7. Additional Context (AGENTS.md): Documents CSS rules for popovers (addressing recurring overflow issues), SSR safety patterns, and popover timing constants for agents.

Type of Change

• Documentation update

Checklist

• I have read the CLAUDE.md guidelines
• Documentation is accurate and reflects existing patterns in the codebase
• No code changes (documentation only)
• No new dependencies added

Notes

This is a documentation-only change that captures existing patterns, constraints, and best practices already in use throughout the codebase. No functional changes or code modifications are included.

https://claude.ai/code/session_0157XtUZgvrxbD6diyJRXQAx

[github-actions]

✅ Playwright Test Report

Status: Tests passed

📊 Download Report & Snapshots (see Artifacts section)

What's in the Visual Snapshots

The gallery includes visual snapshots for:

• 🖥️ Desktop showcase (all variants × all states)
• 📱 Mobile showcase (iPhone SE viewport)
• 📟 Tablet showcase (iPad viewport)
• 🔍 Popover states (verified, partial, not found)
• 🔗 URL citation variants

---

Run ID: 22049432732

[claude]

PR Review: Documentation for Security Patterns, Timing Constants, and Pre-Submission Checklist

Summary

This is a documentation-only PR that captures existing patterns, best practices, and workflow requirements. The documentation is well-structured, accurate, and addresses real issues identified from analyzing previous PRs.

✅ Strengths

1. Security Documentation is Accurate & Valuable

• All documented security utilities exist and match their descriptions:
  • ✅ isDomainMatch() and extractDomain() in src/utils/urlSafety.ts
  • ✅ safeAssign(), isSafeKey(), safeMerge() in src/utils/objectSafety.ts
  • ✅ safeMatch(), safeReplace(), safeTest() in src/utils/regexSafety.ts
  • ✅ sanitizeForLog(), createLogEntry() in src/utils/logSafety.ts
  • ✅ isValidProofImageSrc(), getPortalContainer() in src/react/constants.ts
• Code examples follow correct import patterns (relative paths with .js extensions)
• Security warnings are appropriately stern ("NEVER use X")

2. Timing Constants Are Well-Documented

• All constants exist and values match documentation:
  • ✅ HOVER_CLOSE_DELAY_MS = 150
  • ✅ REPOSITION_GRACE_PERIOD_MS = 300
  • ✅ SPINNER_TIMEOUT_MS = 5000
  • ✅ TOUCH_CLICK_DEBOUNCE_MS = 100
• Test files referenced exist:
  • ✅ src/tests/useRepositionGracePeriod.test.ts
  • ✅ tests/playwright/specs/citationPopoverInteractions.spec.tsx
• The "do not flag as race condition" note is justified given the test coverage

3. Pre-Submission Checklist Is Practical

• ✅ All npm scripts exist (check:fix, lint, build, test, test:ct, size)
• ✅ CI verification: Confirmed .github/workflows/ci.yml runs "npx biome ci ./src"
• ✅ Bundle size limit (15KB for react module) matches package.json size-limit config
• Emphasis on check:fix as the most common failure point is evidence-based

4. Test Coverage Documentation Is Accurate

• ✅ Playwright specs: Verified 9 files in tests/playwright/specs/
• ✅ Popover regression test exists: tests/playwright/specs/popoverImageWidth.spec.tsx
• Unit tests: Found 29 files (minor discrepancy, see below)

5. CSS Rules Address Real Issues

• References PRs Improve not_found chip citation styling and accessibility #243, feat: improve citation component UX with enhanced interactions and visual feedback #244, fix: resolve popover horizontal scrollbar from outer/inner width mismatch #247 — provides concrete context
• max-w-[min(400px,calc(100vw-2rem))] pattern is specific and actionable
• Overflow direction guidance (overflow-y-auto overflow-x-hidden) prevents common mistakes

6. Type Safety Guidelines Are Sound

• Discriminated union completeness rule is a best practice
• "No unsafe casts" guidance with type guard alternative is correct
• Export verification addresses a real pain point (mentioned "follow-up fix PRs")

🔍 Minor Issues

1. Test File Count Discrepancy (AGENTS.md line 198)

• Documentation says "28 files" but there are currently 29 unit test files
• This is a minor documentation drift, likely one test was added after the PR was drafted
• Recommendation: Update to "29 files" or use "~30 files" for future-proofing

2. Missing Context on PR Analysis Scope (AGENTS.md)

• The commit message mentions analyzing "~37 non-dependabot PRs (test: add CitationDrawer tests for XSS prevention, group collapse, and tooltip clamping #200-fix: resolve popover horizontal scrollbar from outer/inner width mismatch #247)"
• This context is not in AGENTS.md itself, which makes the recurring PR references less clear
• Recommendation: Consider adding a brief note about the analysis scope for transparency

🎯 Code Quality Assessment

Best Practices: ✅ Excellent

• Documentation follows the project voice (direct, stern where appropriate, evidence-based)
• Examples use correct TypeScript syntax and import patterns
• Security warnings are appropriately emphatic

Potential Bugs: ✅ None

• This is documentation only, no code changes

Performance: ✅ N/A

• Documentation-only change

Security: ✅ Excellent

• The security patterns section is the strongest part of this PR
• Explicitly documents defense-in-depth utilities that prevent common vulnerabilities

Test Coverage: ✅ Already well-tested

• Documenting existing test patterns, not adding new untested features
• All referenced test files exist

📋 Recommendations

1. Fix the test count: Update "28 files" to "29 files" in AGENTS.md line 198
2. Consider adding context: Briefly mention the PR analysis scope (PRs test: add CitationDrawer tests for XSS prevention, group collapse, and tooltip clamping #200-fix: resolve popover horizontal scrollbar from outer/inner width mismatch #247) for transparency
3. Optional future enhancement: Consider adding a "Common Anti-Patterns" section to CLAUDE.md

✅ Approval

This PR is approved with minor suggestions. The documentation is accurate, well-researched, and addresses real workflow pain points. The test count discrepancy is trivial and can be fixed before merge.

Impact: This will significantly reduce false positives in AI code reviews and help human contributors avoid common security pitfalls.

---

Review generated by Claude Code after verification of:

• Security utility exports and function signatures
• Timing constant values and test coverage
• npm scripts and CI configuration
• Bundle size limits in package.json
• Test file counts and Playwright spec existence
• CSS rules and SSR safety patterns