Skip to content

feat(#986): SOLID 強制を harness に実装 (file-too-large / handler-no-direct-sdk-import + biome complexity) - #1004

Merged
susumutomita merged 1 commit into
mainfrom
feat/solid-harness-rules
May 18, 2026
Merged

susumutomita merged 1 commit into
mainfrom
feat/solid-harness-rules

Conversation

@susumutomita

@susumutomita susumutomita commented May 18, 2026 •

Copy link
Copy Markdown
Owner

Summary

Issue #986 (SOLID 監査 epic) の Phase A: 機械検査の整備。 既存違反を baseline で許容しつつ、 新規追加を block する ratchet 構造を入れる。

  • 新 rule file-too-large (= SRP): .ts/.tsx で 500 行超 = warning、 800 行超 = error
  • 新 rule handler-no-direct-sdk-import (= DIP): handlers//index.ts で @aws-sdk/client-* / lib-* import を warning
  • harness CLI を multi-baseline 対応 (= rule ごとに baselines/.json)
  • 既存違反 19 件を baseline 登録 (file-too-large 11、 handler-no-direct-sdk-import 8)
  • biome に noExcessiveCognitiveComplexity = warn (= 既存 93 関数違反、 段階的 refactor 後 error に上げる)
  • regenerate-baselines.ts CLI で adopt 一発生成

何を解決するか

「ついで」 で 500 行超のファイルや、 SDK を直叩きする handler index.ts を追加させない。 既存違反は #986 の Phase B / C / D で順次解消。 SOLID 5 原則のうち SRP / DIP を機械化し、 OCP / LSP / ISP は後段で 別 rule (例: handler-deps-on-abstraction、 lsp-test-contract) を追加して詰める。

Test plan

  • make harness-test: 20 tests passed (file-too-large 8 + handler-no-direct-sdk-import 7 + 既存 5)
  • make harness: no findings (= baseline 込みで clean)
  • biome check: warn 93 件 (= 期待通り、 error 0、 CI block しない)
  • regenerate-baselines.ts で 2 rule 分の baseline 再生成が動く

Regression 分析

  • 既存 PR 作成パスへの影響なし (= 既存違反は baseline で吸収、 新規違反のみ block)
  • CLI は `adr-self-contained.json` 単体 load → baselines/ ディレクトリ全 load に変更したが、 既存 baseline はそのまま読まれる (= 後方互換)
  • biome の complexity warn は 93 件出るがexitsゼロ (= CI 影響なし)

物理影響

  • CFn: NO-OP
  • CI 時間: harness 552 file × small grep の追加 inspect。 数 ms (= 無視可)
  • biome check 時間: complexity 解析 +N ms

Closes part of #986 (Phase A)

Summary by CodeRabbit

Release Notes

  • New Features

    • Added automated checks to enforce module size and SDK usage standards in handlers.
  • Tests

    • Added comprehensive test suites for architecture validation rules.
  • Chores

    • Improved baseline configuration system for rule management.
    • Enhanced linting standards with cognitive complexity thresholds.

Review Change Stack

…ule を追加 (SOLID 強制)

Why:
  Issue #986 (SOLID 監査 epic) の Phase A として、 architecture harness に
  SRP / DIP の機械検査を入れる。 既存違反は baseline で許容しつつ、 新規追加を block する
  ratchet 構造にして、 1 PR で 「ついで」 に 500 行 / 800 行を超えるファイルや、 handler/index.ts
  からの直 SDK import を増やせなくする。

Changes:
  - file-too-large: .ts/.tsx で 500 行超 = warning、 800 行超 = error
    対象 path: infrastructure/lib/、 apps/*/src/、 scripts/、 packages/*/src/
    .test.ts は除外 (= SRP 別軸)
    match に "ge-500-lines" / "ge-800-lines" bucket を入れ、 1 行増減で baseline match が
    外れない設計 (= 既存違反の細々した行数変動が PR を blockしないように)
  - handler-no-direct-sdk-import: infrastructure/lib/<...>/handlers/<x>/index.ts で
    @aws-sdk/client-* / @aws-sdk/lib-* import を warning
    (= index.ts は HTTP routing 専属、 SDK 呼び出しは service / repository 層に分離する DIP)
    service.ts / shared.ts / 他 non-index は対象外
  - cli.ts: baselines/ ディレクトリの *.json をすべて load して merge する形式に拡張
    既存 adr-self-contained.json と並んで、 rule ごとの baseline ファイルを置けるように
  - baselines/file-too-large.json (11 entries) と
    baselines/handler-no-direct-sdk-import.json (8 entries) を初期登録
    既存違反は Issue #986 Phase B / C / D で順次解消する
  - regenerate-baselines.ts: baseline 再生成 CLI (= adopt rule 時の一発生成)
  - biome.json: noExcessiveCognitiveComplexity = warn (max 15)
    既存 93 関数が違反しているため warn (= CI block しない)、 段階的に refactor して
    将来 error に格上げ。 spirit は CLAUDE.md の 「config を緩めて誤魔化さない」 ではなく
    「新規 enforcement の追加」 なので config 編集を許容。

Test:
  - file-too-large.test.ts (8 ケース) + handler-no-direct-sdk-import.test.ts (7 ケース)
  - make harness-test: 20 tests passed
  - make harness: no findings

Regression 分析:
  - 既存 PR 作成パスへの影響なし (= warn / 既存違反は baseline で吸収)
  - 新規に 500 行超のファイルを足す PR は warning で見える化、 800 行は error で停止
  - 新規 handler index.ts で SDK 直 import する PR は warning で見える化

物理影響:
  - CFn: NO-OP
  - CI: harness rule 追加分の inspect 時間 +N ms (= 552 file × small grep、 無視可)

Closes part of #986 (= SOLID 監査 epic Phase A)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented May 18, 2026 •

Copy link
Copy Markdown

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

This pull request adds two new architecture linting rules to enforce code organization: file-too-large detects oversized modules exceeding 500 or 800 lines, and handler-no-direct-sdk-import prevents direct AWS SDK imports in HTTP handler entry points. A baseline system tracks known violations to prevent alert fatigue, with a regeneration tool to manage baseline entries and updated CLI integration to load and apply baselines during harness execution.

Changes

Architecture Rules and Baseline System

Layer / File(s) Summary
File-too-large rule
\.claude/harness/src/rules/file-too-large.ts, \.claude/harness/src/rules/file-too-large.test.ts, \.claude/harness/baselines/file-too-large.json
fileTooLarge rule emits warnings (≥500 lines) or errors (≥800 lines) for TypeScript files in specified path prefixes, excluding tests and vendor directories. Findings use bucketed match identifiers (ge-500-lines, ge-800-lines) to remain stable across minor line-count changes. Comprehensive tests cover threshold boundaries, path filtering, and baseline match stability.
Handler SDK import rule
\.claude/harness/src/rules/handler-no-direct-sdk-import.ts, \.claude/harness/src/rules/handler-no-direct-sdk-import.test.ts, \.claude/harness/baselines/handler-no-direct-sdk-import.json
handlerNoDirectSdkImport rule detects and reports direct @aws-sdk/client-* and @aws-sdk/lib-* imports in infrastructure/lib/**/handlers/**/index.ts handler files. Recommends moving SDK calls to service/repository modules to decouple routing from infrastructure. Tests verify import detection, handler-layer filtering, and multiple-import reporting with stable match values.
Baseline regeneration tool and loading
\.claude/harness/bin/regenerate-baselines.ts, \.claude/harness/src/cli.ts (loadAllBaselines)
Introduces regenerate-baselines.ts CLI tool that runs specified architecture rules and normalizes findings into baseline JSON entries with rule ID, file path, line number, and bucketed match identifiers. loadAllBaselines(dir) reads all *.json files from the baselines directory, tolerates missing directories, and merges entries into a unified suppression set.
Harness CLI: rule registration and baseline loading
\.claude/harness/src/cli.ts, \.claude/harness/src/rules/index.ts
Registers fileTooLarge and handlerNoDirectSdkImport rules in CLI imports, ALL_RULES list, and rules/index exports. Updates run() to load all baseline JSON files via merged baseline set, then suppress findings via isBaselined against merged entries. Extends HELP_TEXT with baseline organization, suppression semantics, and regeneration command documentation.
Linting configuration
biome.json
Adds noExcessiveCognitiveComplexity rule set to warning level with maxAllowedComplexity threshold of 15.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 Two rules now guard the code with care,
One counts the lines, the other SDK snares,
Baselines baseline what we already know,
So warnings won't pile high—just let good findings flow! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% 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 PR title clearly identifies the main change: implementing SOLID enforcement (file-too-large and handler-no-direct-sdk-import rules) in the harness, with biome complexity configuration added.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ 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 feat/solid-harness-rules

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@susumutomita
susumutomita enabled auto-merge (squash) May 18, 2026 02:58
@susumutomita
susumutomita merged commit 40ddc53 into main May 18, 2026
7 of 8 checks passed
@susumutomita
susumutomita deleted the feat/solid-harness-rules branch May 18, 2026 02:59
@codecov

codecov Bot commented May 18, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (main@fb4cb2a). Learn more about missing BASE report.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1004   +/-   ##
=======================================
  Coverage        ?   92.95%           
=======================================
  Files           ?        8           
  Lines           ?      227           
  Branches        ?       66           
=======================================
  Hits            ?      211           
  Misses          ?       16           
  Partials        ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

1 participant