refactor: replace module-level mutable state with per-analysis RuleContext state

[let-sunny]

Summary

• Adds analysisState: Map<string, unknown> to RuleContext and a typed getAnalysisState<T>() helper for lazy-init per-analysis state
• Removes 3 module-level Sets and 2 reset*() functions from component/index.ts
• rule-engine.ts creates a fresh Map per analyze() call — no more manual reset contracts
• Updates all test files to use fresh analysisState per test instead of calling reset functions

Closes #46

Why this is the top priority for v1

See comment on #46 for the full rationale.

Test plan

• All 387 existing tests pass
• TypeScript type check passes (pnpm lint)
• Dedup behavior verified: fresh analysisState = fresh dedup state (tested explicitly)
• MCP server safety: no module-level state to leak between analysis runs

https://claude.ai/code/session_01JYHYR7hwuZ7cLwrJadAcVz

Summary by CodeRabbit

• Bug Fixes

  • Analysis runs are now isolated so results no longer leak between runs, preventing stale or missing issue reports.

• Refactor

  • State management moved to per-analysis scope so manual resets are no longer required.

• Tests

  • Added tests that verify consistent, repeatable results across consecutive analyses with the same input.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: e7583c3e-8433-4d18-9ebf-bc19829819a2

📥 Commits

Reviewing files that changed from the base of the PR and between f780b8b and d5b6884.

📒 Files selected for processing (2)

• src/core/contracts/rule.ts
• src/core/engine/rule-engine.test.ts

---

📝 Walkthrough

Walkthrough

Rule execution state was moved from module-level globals to a per-analysis Map injected on RuleContext. getAnalysisState was added; RuleEngine now creates and threads analysisState through traversal and rule checks. Module-level reset exports were removed and tests updated to supply per-run state.

Changes

Cohort / File(s)	Summary
Contracts src/core/contracts/rule.ts	Added analysisState: Map<string, unknown> to RuleContext and exported getAnalysisState<T>(context, key, init) helper.
Engine src/core/engine/rule-engine.ts	Create per-analysis analysisState in RuleEngine.analyze, pass it into traverseAndCheck and include it on each RuleContext; removed module-level reset calls.
Component rule implementation src/core/rules/component/index.ts	Replaced module-level Set dedup with per-analysis Sets stored via getAnalysisState; removed exported reset functions.
Component rule tests src/core/rules/component/index.test.ts, src/core/rules/component/missing-component.test.ts	Removed reliance on reset exports; tests now inject a fresh analysisState Map per test to simulate new analysis runs.
Other rule tests / helpers src/core/rules/ai-readability/invisible-layer.test.ts, src/core/rules/custom/custom-rule-loader.test.ts	Updated makeContext() test helpers to include analysisState: new Map() (and componentDepth where applicable).
New tests src/core/engine/rule-engine.test.ts	Added tests that run RuleEngine.analyze twice with the same engine instance to assert per-analysis state isolation for "missing-component" issues.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant RuleEngine
  participant Traverser
  participant Rule as Rule/RuleContext

  Client->>RuleEngine: analyze(AnalysisFile)
  Note right of RuleEngine: creates analysisState = Map()
  RuleEngine->>Traverser: traverseAndCheck(rootNode, analysisState)
  Traverser->>Rule: build RuleContext (includes analysisState)
  Note right of Rule: rule.check(context)
  Rule->>Rule: getAnalysisState(context, key, init) — create/read per-analysis Set
  Rule->>Traverser: (recursive) traverse children with same analysisState
  Traverser->>RuleEngine: return collected issues
  RuleEngine->>Client: analysis results

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• refactor: unify missing-component with 4-stage checks (#17) #22: Introduced the previous module-level dedup state and exported reset functions that this change replaces with per-analysis analysisState and getAnalysisState.

Poem

🐰 I hopped through maps and sets today,
Per-run little nests keep bugs away.
No globals lingering in the sun—
Fresh maps for each analysis run! 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 8.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely summarizes the main change: replacing module-level mutable state with per-analysis RuleContext state, which is the core refactoring throughout the PR.
Linked Issues check	✅ Passed	The PR successfully implements all coding requirements from issue #46: eliminates module-level Sets, adds analysisState to RuleContext with getAnalysisState helper, updates rule-engine.ts to create per-analysis state, and removes reset functions.
Out of Scope Changes check	✅ Passed	All changes are within scope: contract updates (RuleContext, getAnalysisState), rule-engine modifications, component rule refactoring, and test updates to use per-analysis state instead of resets.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch claude/v1-release-prep-2rBCk

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/core/engine/rule-engine.ts (1)

147-179: 🧹 Nitpick | 🔵 Trivial

Add a regression that goes through RuleEngine.analyze() twice.

This is the code path that guarantees a fresh analysisState per run. A small integration test that calls analyze() twice on the same RuleEngine instance and expects the same component-dedup violation both times would lock in the production bugfix.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/core/engine/rule-engine.ts` around lines 147 - 179, Add an integration
test that exercises RuleEngine.analyze() twice on the same RuleEngine instance
to ensure analysisState is fresh per run: instantiate a RuleEngine with the same
test document, call analyze(file) once and capture issues, call analyze(file) a
second time and assert the same component-dedup violation (or identical issues)
is produced; target the RuleEngine.analyze method and the traversal path through
traverseAndCheck/analysisState to reproduce any state-leak regression.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/core/contracts/rule.ts`:
- Around line 53-58: getAnalysisState currently treats a cached undefined the
same as a missing key because it checks the retrieved value (`existing !==
undefined`) instead of whether the Map contains the key; update getAnalysisState
to call context.analysisState.has(key) to detect presence, and only call
context.analysisState.get(key) when has(...) is true, otherwise run init(), set
the result with context.analysisState.set(key, value) and return it so an
initializer that returns undefined is preserved in the cache; references:
function getAnalysisState, property analysisState on RuleContext, parameter key
and init.

---

Outside diff comments:
In `@src/core/engine/rule-engine.ts`:
- Around line 147-179: Add an integration test that exercises
RuleEngine.analyze() twice on the same RuleEngine instance to ensure
analysisState is fresh per run: instantiate a RuleEngine with the same test
document, call analyze(file) once and capture issues, call analyze(file) a
second time and assert the same component-dedup violation (or identical issues)
is produced; target the RuleEngine.analyze method and the traversal path through
traverseAndCheck/analysisState to reproduce any state-leak regression.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: a826e6bc-a1b3-4392-b232-261a8cbe8f08

📥 Commits

Reviewing files that changed from the base of the PR and between a5ea872 and f780b8b.

📒 Files selected for processing (7)

• src/core/contracts/rule.ts
• src/core/engine/rule-engine.ts
• src/core/rules/ai-readability/invisible-layer.test.ts
• src/core/rules/component/index.test.ts
• src/core/rules/component/index.ts
• src/core/rules/component/missing-component.test.ts
• src/core/rules/custom/custom-rule-loader.test.ts