[GH-ISSUE #151] Technical debt: high complexity/large files from lint warnings

bsakaguchi commented

2026-10-15 18:21:06 +00:00

Owner

Originally created by @morluto on GitHub (Jul 14, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/151

Lint now reports many technical-debt hotspots with no behavior failures:

src/application/BinarySession.ts:647 file has 577 lines (>500)
src/domain/errors.ts:825 file has 729 lines plus complexity 52 and multiple max-params hits
src/application/CapabilityInventory.ts:88 max-params (6)
src/application/Unknown? none in repo (new work includes no issues there)
src/server/registerEvidenceResources.ts:17 function length 283
src/application/ArtifactInventory.ts and src/server/registerSessionTools.ts have repeated oversized functions

This is mostly maintainability risk (hard-to-review logic, high regression cost) and should be tracked.

Suggested follow-up:

Split oversized functions in BinarySession, ArtifactInventory, and registerSessionTools.
Reduce complexity in src/domain/errors.ts and src/application/Doctor.ts to simplify future invariants.
Add debt ticket breakdown by file for staged cleanup.

Originally created by @morluto on GitHub (Jul 14, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/151 Lint now reports many technical-debt hotspots with no behavior failures: - `src/application/BinarySession.ts:647` file has 577 lines (>500) - `src/domain/errors.ts:825` file has 729 lines plus complexity 52 and multiple max-params hits - `src/application/CapabilityInventory.ts:88` max-params (6) - `src/application/Unknown?` none in repo (new work includes no issues there) - `src/server/registerEvidenceResources.ts:17` function length 283 - `src/application/ArtifactInventory.ts` and `src/server/registerSessionTools.ts` have repeated oversized functions This is mostly maintainability risk (hard-to-review logic, high regression cost) and should be tracked. Suggested follow-up: - Split oversized functions in `BinarySession`, `ArtifactInventory`, and `registerSessionTools`. - Reduce complexity in `src/domain/errors.ts` and `src/application/Doctor.ts` to simplify future invariants. - Add debt ticket breakdown by file for staged cleanup.

bsakaguchi

2026-10-15 18:21:06 +00:00

closed this issue
added the
P3: low

triage

enhancement
labels

bsakaguchi commented

2026-10-15 18:21:08 +00:00

Author

Owner

@morluto commented on GitHub (Jul 17, 2026):

Refreshed against current main (2026-07-17).

npm run lint completes successfully but reports a much larger, changed hotspot set than the original body. The most actionable clusters are:

error projection: src/domain/errors.ts (976 lines; complexity 64/32/21/22 in projection functions);
environment diagnosis: src/application/Doctor.ts (620 lines; runDoctor complexity 67 and 268 lines);
managed metadata: ManagedMemberInspector.ts (1,447 lines), ManagedMetadataLayout.ts (complexity 46/25), plus related inventory/boundary modules;
adapter composition: src/cli.ts, registerEvidenceResources.ts, registerManagedWorkflowTools.ts, and session/application registration functions;
focused domain modules such as reconstruction coverage and managed application graphs that now exceed the file threshold.

Several locations named in the original body have moved or changed, so the old line numbers should not be treated as current evidence. Recommended scope is to keep this issue as an inventory and address separate cohesive clusters through behavior-preserving refactors. Highest leverage: (1) split typed error-to-message/remediation projection by error family, (2) separate doctor probes from result assembly, and (3) split managed metadata table/member parsing around existing domain boundaries. Avoid file splitting that merely moves lines without reducing caller burden or concentrating policy.

@morluto commented on GitHub (Jul 17, 2026): Refreshed against current main (2026-07-17). `npm run lint` completes successfully but reports a much larger, changed hotspot set than the original body. The most actionable clusters are: - error projection: `src/domain/errors.ts` (976 lines; complexity 64/32/21/22 in projection functions); - environment diagnosis: `src/application/Doctor.ts` (620 lines; `runDoctor` complexity 67 and 268 lines); - managed metadata: `ManagedMemberInspector.ts` (1,447 lines), `ManagedMetadataLayout.ts` (complexity 46/25), plus related inventory/boundary modules; - adapter composition: `src/cli.ts`, `registerEvidenceResources.ts`, `registerManagedWorkflowTools.ts`, and session/application registration functions; - focused domain modules such as reconstruction coverage and managed application graphs that now exceed the file threshold. Several locations named in the original body have moved or changed, so the old line numbers should not be treated as current evidence. Recommended scope is to keep this issue as an inventory and address separate cohesive clusters through behavior-preserving refactors. Highest leverage: (1) split typed error-to-message/remediation projection by error family, (2) separate doctor probes from result assembly, and (3) split managed metadata table/member parsing around existing domain boundaries. Avoid file splitting that merely moves lines without reducing caller burden or concentrating policy.

bsakaguchi commented

2026-10-15 18:21:08 +00:00

Author

Owner

@morluto commented on GitHub (Jul 17, 2026):

Incremental refactor on the current working tree: CapabilityInventory now separates policy, target, and provider availability decisions behind one readonly context. This removes its prior complexity=30, max-lines-per-function, and max-params warnings without changing inventory output or ordering. Focused oxlint is clean; typecheck and the four server identity/capability inventory tests pass. The broader warning inventory remains open.

@morluto commented on GitHub (Jul 17, 2026): Incremental refactor on the current working tree: CapabilityInventory now separates policy, target, and provider availability decisions behind one readonly context. This removes its prior complexity=30, max-lines-per-function, and max-params warnings without changing inventory output or ordering. Focused oxlint is clean; typecheck and the four server identity/capability inventory tests pass. The broader warning inventory remains open.

bsakaguchi commented

2026-10-15 18:21:08 +00:00

Author

Owner

@morluto commented on GitHub (Jul 17, 2026):

Focused debt reduction: CLI and MCP process capture now share one pure processCapturePermissionRequest builder instead of maintaining duplicate policy projections. The builder preserves roots, executable, environment-name, network, mount, and operation-identity semantics and has direct contract coverage. This also gives #96 one canonical request to commit into future signed elicitation state. Full npm run check passes on the final tree. #151 remains open for the measured hotspots, especially the error-projection and Doctor complexity clusters.

@morluto commented on GitHub (Jul 17, 2026): Focused debt reduction: CLI and MCP process capture now share one pure processCapturePermissionRequest builder instead of maintaining duplicate policy projections. The builder preserves roots, executable, environment-name, network, mount, and operation-identity semantics and has direct contract coverage. This also gives #96 one canonical request to commit into future signed elicitation state. Full npm run check passes on the final tree. #151 remains open for the measured hotspots, especially the error-projection and Doctor complexity clusters.

bsakaguchi commented

2026-10-15 18:21:09 +00:00

Author

Owner

@morluto commented on GitHub (Jul 17, 2026):

Refreshed against the current worktree on 2026-07-17.

The inventory remains valid, but the original line references and several measurements are stale:

src/domain/errors.ts: 976 measured lines; error projection remains the highest-leverage split, with complexity 64/32/21/22 hotspots.
src/application/BinarySession.ts: 807 measured lines; open remains complexity 30 and execute complexity 24.
src/server/registerEvidenceResources.ts: the registration function is now 324 lines.
src/application/CapabilityInventory.ts: the prior max-params warning is gone in the current worktree, but knip reports ToolAvailabilityReason as the only unused exported type.
src/application/Doctor.ts: runDoctor is now a major hotspot (complexity 67, 268 lines).

Recommended next focused slice: split error projection by error family into lookup-backed helpers, preserving the tagged error algebra and existing exhaustive tests. The issue should stay open.

@morluto commented on GitHub (Jul 17, 2026): Refreshed against the current worktree on 2026-07-17. The inventory remains valid, but the original line references and several measurements are stale: - `src/domain/errors.ts`: 976 measured lines; error projection remains the highest-leverage split, with complexity 64/32/21/22 hotspots. - `src/application/BinarySession.ts`: 807 measured lines; `open` remains complexity 30 and `execute` complexity 24. - `src/server/registerEvidenceResources.ts`: the registration function is now 324 lines. - `src/application/CapabilityInventory.ts`: the prior max-params warning is gone in the current worktree, but knip reports `ToolAvailabilityReason` as the only unused exported type. - `src/application/Doctor.ts`: `runDoctor` is now a major hotspot (complexity 67, 268 lines). Recommended next focused slice: split error projection by error family into lookup-backed helpers, preserving the tagged error algebra and existing exhaustive tests. The issue should stay open.

bsakaguchi commented

2026-10-15 18:21:09 +00:00

Author

Owner

@morluto commented on GitHub (Jul 17, 2026):

Implemented the recommended focused error-projection slice on the current working tree.

src/domain/errors.ts now routes code and details projection through small error-family helpers plus a static tag-to-code table. Caller behavior is preserved: an independent Devin CLI review compared projections from HEAD and the working tree across the concrete error variants and reason branches and found no output differences; the repository's exhaustive projection tests also pass.

Measured effect:

errorCode: complexity 64 and 118-line function warnings removed.
errorDetails: complexity 32 and 105-line function warnings removed.
focused errors.ts warnings: 8 -> 4.
remaining warnings: file size, PermissionRequiredError constructor arity, errorCategory complexity 21, and userMessage complexity 22.

Validation: npm run check passes on the full current tree (typecheck, lint, format, generated docs/schema checks, and all 160 Vitest files). The issue remains open; the next cohesive error-algebra slice is to separate category/message policy without weakening exhaustiveness, while Doctor remains the largest unrelated complexity hotspot.

@morluto commented on GitHub (Jul 17, 2026): Implemented the recommended focused error-projection slice on the current working tree. `src/domain/errors.ts` now routes code and details projection through small error-family helpers plus a static tag-to-code table. Caller behavior is preserved: an independent Devin CLI review compared projections from `HEAD` and the working tree across the concrete error variants and reason branches and found no output differences; the repository's exhaustive projection tests also pass. Measured effect: - `errorCode`: complexity 64 and 118-line function warnings removed. - `errorDetails`: complexity 32 and 105-line function warnings removed. - focused `errors.ts` warnings: 8 -> 4. - remaining warnings: file size, `PermissionRequiredError` constructor arity, `errorCategory` complexity 21, and `userMessage` complexity 22. Validation: `npm run check` passes on the full current tree (typecheck, lint, format, generated docs/schema checks, and all 160 Vitest files). The issue remains open; the next cohesive error-algebra slice is to separate category/message policy without weakening exhaustiveness, while `Doctor` remains the largest unrelated complexity hotspot.

bsakaguchi commented

2026-10-15 18:21:09 +00:00

Author

Owner

@morluto commented on GitHub (Jul 17, 2026):

Follow-up after independent review: Devin correctly identified that the first lookup-table draft used Partial<Record<AnalysisErrorTag, ...>>, weakening the old switch's compile-time exhaustiveness. The final version now defines the specialized tag set explicitly and requires a complete Record<Exclude<AnalysisErrorTag, SpecializedErrorTag>, ...>; adding a new ordinary error tag without a code mapping fails typecheck.

I also completed the adjacent category/message split. Focused errors.ts warnings are now 8 -> 2, not 8 -> 4: all four complexity warnings and both oversized projection-function warnings are gone. The two remaining warnings are the file-level size and the five-argument PermissionRequiredError constructor. Typecheck, the focused five-test error suite, focused oxlint, and git diff --check pass after the follow-up.

@morluto commented on GitHub (Jul 17, 2026): Follow-up after independent review: Devin correctly identified that the first lookup-table draft used `Partial<Record<AnalysisErrorTag, ...>>`, weakening the old switch's compile-time exhaustiveness. The final version now defines the specialized tag set explicitly and requires a complete `Record<Exclude<AnalysisErrorTag, SpecializedErrorTag>, ...>`; adding a new ordinary error tag without a code mapping fails typecheck. I also completed the adjacent category/message split. Focused `errors.ts` warnings are now **8 -> 2**, not 8 -> 4: all four complexity warnings and both oversized projection-function warnings are gone. The two remaining warnings are the file-level size and the five-argument `PermissionRequiredError` constructor. Typecheck, the focused five-test error suite, focused oxlint, and `git diff --check` pass after the follow-up.

bsakaguchi commented

2026-10-15 18:21:09 +00:00

Author

Owner

@morluto commented on GitHub (Jul 17, 2026):

Completed the focused Doctor diagnostics refactor on the current working tree.

The read-only workflow now separates ordered environment/provider/optional-target probes from installed skill, registration, and report assembly. Exact probe and check ordering is preserved, including the Linux Hopper version -> provider -> replay -> ILSpy -> demo-runtime -> target sequence.

Measured effect:

runDoctor: 268 lines -> 28 lines;
the prior complexity 67 warning is removed;
the prior runDoctor max-lines-per-function warning is removed;
Doctor.ts now retains only the pre-existing 164-line systemDoctorHost warning.

Validation: focused Doctor and Ghidra Doctor suites pass (27/27), and the final npm run docs:generate && npm run check passes across 160 test files and 1,089 tests. Generated TypeDoc is synchronized. #151 remains open for the broader measured inventory, with systemDoctorHost, BinarySession, managed metadata, adapter composition, and other lint hotspots still outstanding.

@morluto commented on GitHub (Jul 17, 2026): Completed the focused Doctor diagnostics refactor on the current working tree. The read-only workflow now separates ordered environment/provider/optional-target probes from installed skill, registration, and report assembly. Exact probe and check ordering is preserved, including the Linux Hopper version -> provider -> replay -> ILSpy -> demo-runtime -> target sequence. Measured effect: - runDoctor: 268 lines -> 28 lines; - the prior complexity 67 warning is removed; - the prior runDoctor max-lines-per-function warning is removed; - Doctor.ts now retains only the pre-existing 164-line systemDoctorHost warning. Validation: focused Doctor and Ghidra Doctor suites pass (27/27), and the final npm run docs:generate && npm run check passes across 160 test files and 1,089 tests. Generated TypeDoc is synchronized. #151 remains open for the broader measured inventory, with systemDoctorHost, BinarySession, managed metadata, adapter composition, and other lint hotspots still outstanding.

bsakaguchi commented

2026-10-15 18:21:09 +00:00

Author

Owner

@morluto commented on GitHub (Jul 18, 2026):

The Doctor hotspot refactor is now proposed in #305: runDoctor is reduced to orchestration over DoctorDiagnostics, and shared error projections are decomposed into typed helpers. Full repository and package validation pass. The remaining measured warning is the separate systemDoctorHost host-construction function, so the broader inventory stays open.

@morluto commented on GitHub (Jul 18, 2026): The Doctor hotspot refactor is now proposed in #305: `runDoctor` is reduced to orchestration over `DoctorDiagnostics`, and shared error projections are decomposed into typed helpers. Full repository and package validation pass. The remaining measured warning is the separate `systemDoctorHost` host-construction function, so the broader inventory stays open.

bsakaguchi commented

2026-10-15 18:21:10 +00:00

Author

Owner

@morluto commented on GitHub (Jul 20, 2026):

Split into focused child issues:

#394

These replace the broad remaining scope in this issue. Closing this parent can happen once all children are closed.

@morluto commented on GitHub (Jul 20, 2026): Split into focused child issues: - [ ] #394 These replace the broad remaining scope in this issue. Closing this parent can happen once all children are closed.

bsakaguchi commented

2026-10-15 18:21:10 +00:00

Author

Owner

@morluto commented on GitHub (Jul 20, 2026):

Closing as no longer actionable. npm run lint reports 0 warnings/errors and npm run jscpd shows only 1.65% duplicated lines, so these metric-driven refactors would force harmful cross-layer abstractions without improving real correctness.

@morluto commented on GitHub (Jul 20, 2026): Closing as no longer actionable. `npm run lint` reports 0 warnings/errors and `npm run jscpd` shows only 1.65% duplicated lines, so these metric-driven refactors would force harmful cross-layer abstractions without improving real correctness.

bsakaguchi referenced this issue

2026-10-15 18:21:45 +00:00

[GH-ISSUE #267] Add approval-gated managed-runtime characterization #156

bsakaguchi referenced this issue

2026-10-15 18:22:47 +00:00

[GH-ISSUE #390] Select and pin MCP Tasks extension revision and build retained-operation state machine #236

bsakaguchi referenced this issue

2026-10-15 18:22:51 +00:00

[GH-ISSUE #391] Add MCP Tasks conformance adapter #239

bsakaguchi referenced this issue

2026-10-16 21:11:45 +00:00

[PR #125] [MERGED] feat(mcp): expose progress, resources, and availability #1848