diff --git a/.github/workflows/scripts/version-manager.ts b/.github/workflows/scripts/version-manager.ts index 5dab699cdb..54a682c31f 100644 --- a/.github/workflows/scripts/version-manager.ts +++ b/.github/workflows/scripts/version-manager.ts @@ -241,7 +241,6 @@ class VersionManager { } // CLI interface -// @ts-ignore if (import.meta.main) { const [command, ...args] = process.argv.slice(2) const manager = new VersionManager() diff --git a/.github/workflows/test_visualization.yml b/.github/workflows/test_visualization.yml index 0f8ce6f7e6..6012f1659c 100644 --- a/.github/workflows/test_visualization.yml +++ b/.github/workflows/test_visualization.yml @@ -55,6 +55,12 @@ jobs: npm run lint:architecture || (echo "❌ Architecture violations detected! Check the output above for details." && exit 1) working-directory: ${{env.working-directory}} + - name: Check component styles + run: | + echo "Checking for component style files (no-component-scss-files)..." + npm run lint:styles || (echo "❌ Component style files detected! Use Tailwind utility classes instead." && exit 1) + working-directory: ${{env.working-directory}} + - name: Check dead code run: | echo "Checking for unused exports/files (knip)..." diff --git a/.gitignore b/.gitignore index 0ae5cebf8c..76d358eb65 100644 --- a/.gitignore +++ b/.gitignore @@ -2,8 +2,17 @@ codecharta-analyse.sonargraph auth.txt .cache +# brand reference — internal corporate palette, must not be committed +Ideas/brand.png # call-graph exports (e.g. analysis.cg.json) — large, generated *.cg.json +# analyser output written at the analysis root (e.g. dlc.cc.json.gz) — generated, not a tracked fixture +/analysis/*.cc.json +/analysis/*.cc.json.gz +# analyser output dropped at the repo root (e.g. maibornwolff-domain.cc.json) — generated, and biome +# would otherwise try to format the minified map. Tracked fixtures all live in subdirectories. +/*.cc.json +/*.cc.json.gz **/node-wrapper/public .scannerwork diff --git a/CLAUDE.md b/CLAUDE.md index 0e9c26933a..5cd3526f46 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -291,14 +291,18 @@ Example: `feat(visualization): add dark mode toggle (#123)` - Based on official Kotlin Coding Conventions - Auto-formatted via `./gradlew ktlintFormat` - Rules defined in `.editorconfig` -- **Function syntax**: Use block-body style with braces `{ }` consistently, not expression-body style with `=` +- **Function syntax**: Follow ktlint (`ktlint_code_style = ktlint_official`), which requires an + expression body (`=`) for a function whose body is a single expression, and a block body `{ }` for + everything else. `./gradlew ktlintFormat` rewrites this for you — do not hand-write the other form, + `ktlintCheck` fails it. - **Guard clauses**: Use early returns for error conditions and edge cases to reduce nesting - **If expressions**: Prefer concise single-line style when possible: - ✅ `val x = if (condition) valueA else valueB` - ❌ `val x = if (condition) { valueA } else { valueB }` - Use multi-line only when branches contain multiple statements or complex logic - **Magic strings/numbers**: Extract repeated literals to constants in `companion object` -- **Function organization**: Group related functions with section comments +- **Function organization**: Keep related functions next to each other; order them top-down (callers + before callees) instead of labelling groups with section comments - **Parameter naming**: Use consistent, descriptive names across related functions **Visualization (TypeScript)**: @@ -322,7 +326,12 @@ Example: `feat(visualization): add dark mode toggle (#123)` - **Expressive Naming**: Descriptive names that reveal intent (>2 chars, avoid abbreviations) - **Fix Warnings**: Never suppress, always resolve - **Consistent Style**: Match existing patterns; follow language conventions -- **Comments**: Use sparingly; explain *why* not *what*. Prefer clear function names over comments. +- **Comments**: Default to **no comment**. Write one only when the code cannot be made to explain + itself and a reader would otherwise draw a wrong conclusion — a non-obvious constraint, a + workaround (reference the issue), a deliberate trade-off. Never restate what the code does. + Naming is the tool: rename the method, extract a named helper, introduce a named constant or + intermediate variable instead of writing a sentence. Prose blocks that narrate a class, a + template section, or a sequence of steps do not belong in the code — delete them. - **Method Size**: Keep methods <25 lines; split complex logic into focused helpers - **Parameter Lists**: Max 3-4 parameters; use data classes for related groups - **Metric Accuracy**: All metrics must be deterministic and reproducible across runs diff --git a/CODE_QUALITY.md b/CODE_QUALITY.md index 7e2a07f0c3..0c9cd8adbf 100644 --- a/CODE_QUALITY.md +++ b/CODE_QUALITY.md @@ -93,7 +93,10 @@ private fun writeOutput(data: TransformedData): Result { ... } ### 4. Comments -**RULE:** Comments explain *why*, never *what*. Code should be so clear that *what* is obvious. +**RULE:** The default is **no comment**. Make the code explain itself through naming — rename the +method, extract a named helper, introduce a named constant or intermediate variable. Write a comment +only when that is impossible and a reader would otherwise draw a wrong conclusion; then it explains +*why*, never *what*. **❌ BAD:** ```kotlin @@ -119,6 +122,8 @@ val compatible = projects.filter { Project.isAPIVersionCompatible(it.apiVersion) - ✅ Performance trade-offs - ❌ Explaining obvious code - ❌ Commented-out code (delete it) +- ❌ Prose that narrates a class, a template section, or a sequence of steps — rename and extract instead +- ❌ Doc comments (KDoc/JSDoc) that only repeat the signature ### 5. Function Parameters diff --git a/REVIEW-branch-6lens.md b/REVIEW-branch-6lens.md new file mode 100644 index 0000000000..4f9322fb91 --- /dev/null +++ b/REVIEW-branch-6lens.md @@ -0,0 +1,593 @@ +# Branch Review — `feature/domainlanguage-parser` + +**Scope:** 12 commits vs `main` · 401 files · +23,512 / −971 · new Kotlin `DomainLanguageParser`, new Domain View + word cloud, sidebar-explorer refactor. +**Method:** 6 reviewer lenses, each fanned out over 4 subsystems (analysis parser · domain feature · explorer refactor · cross-cutting infra), synthesized per lens, then every high-severity technical finding adversarially verified. Plus the real frontend tooling gates. + +--- + +## TL;DR + +| Lens | Grade | +|---|---| +| Clean Code | **B** | +| SOLID | **B−** | +| DRY | **C** | +| Architecture | **B−** | +| Comment Hygiene | **B+** | +| Brutal Roast | **C** | + +**The one thing to fix before merge — a real functional bug (verified by hand):** +Four `DomainState` settings — **`DrawOutOfBound`, `ShrinkToFit`, `SortingOrder`, `SortingOrderAscending`** — are restored on load but **never persisted**. `actionsRequiringSaveCcState.ts` registers a save-trigger for only **7** of the 11 domain setters that `loadInitialFile.store.ts` dispatches on restore. The per-view sort order (the feature this branch's last commits added) silently does not survive a reload. +→ `visualization/app/codeCharta/load/effects/saveCcState/actionsRequiringSaveCcState.ts` vs `visualization/app/codeCharta/load/loadInitialFile.store.ts:258` + +**Everything else is quality/design, not correctness.** No critical defects. The recurring story across all lenses: *competent code buried under ceremony* — 11 folder-per-primitive ngrx slices, three parallel hand-maintained registries (persist / restore / merge) that have already drifted, and load-bearing logic copied instead of reusing helpers that already exist and are exported. + +--- + +## Frontend Tooling Gates (actually run on the branch) + +| Gate | Result | +|---|---| +| **Unit tests** (`jest`) | ✅ 376 suites / 2546 tests green (2544 pass, 2 todo), 45 snapshots | +| **Knip** (dead code / unused deps) | ✅ clean | +| **lint:styles** (custom) | ✅ clean | +| **Biome** | ❌ 3 auto-fixable `organizeImports` errors — `domainState.read.facade.ts`, `sidebarExplorer/facade.ts`, `explorerSortControl.component.spec.ts` (slipped past the pre-commit hook; `npm run format` clears them) | +| **dependency-cruiser** (`lint:architecture`) | ❌ 3 boundary violations — `navBar`, `fileExtensionBar`, `bottomBar` import `features/shared/.../publishesHeight.directive.ts` **directly** instead of via the shared public API barrel (`feature-cross-feature-only-via-public-api`) | + +--- + +## Verification Highlights + +- ✅ **Persistence-drop bug (SOLID/high)** — CONFIRMED by hand: 7 setters saved vs 11 restored. +- ❌ **"cc.json 2.0 schema duplicated in two files will drift" (DRY/high)** — FALSE POSITIVE: `ccJson2Schema.drift.spec.ts` already asserts `toEqual` between the vendored copy and `dev_docs/cc-json-2.0.schema.json`. The duplication is real but guarded. +- The generic-explorer coupling, `domainWords` persistence-asymmetry, and twin-selector findings were all judged technically accurate but **overstated → low** (real coupling, minor impact). + +## Adversarial Verification of High-Severity Technical Findings + +Every `high`/`critical` non-roast finding was independently re-checked by a skeptic agent instructed to refute it. Net across both verify passes: + +- **OVERSTATED → low** — [solid] WordCloudComponent violates its own "reusable presentational component" contract by hard-wiring the domain view + - `visualization/app/codeCharta/renderer/wordCloud/components/wordCloud/wordCloud.component.ts:81` + - Every literal claim in the finding checks out: the file is new on this branch; line 81 injects the routing-layer ViewReadinessStore, line 80 injects WordCloudReadStore, lines 152/191 hardcode markReady("domain"), and the docstring at 67-70 uses the phrase "reusable presentational component." The DIP/SRP coupling critique is legitimate — a renderer reaching up into an app-global routing store and baking in a caller-specific "domain" string is genuine cross-layer coupling. + +However, "high" severity is not justified. (1) The docstring's "reusable presentational component" claim is explicitly and +- **FALSE_POSITIVE → none** — [dry] cc.json 2.0 JSON schema maintained verbatim in two files + - `visualization/app/codeCharta/util/ccJson2Schema.json:47` + - The two schema files are genuinely byte-identical and both are edited on this branch — that observation is literally true. However, the finding's entire severity rests on the claim that they "will silently drift the first time only one is edited," and that is false. A dedicated guard test, visualization/app/codeCharta/util/ccJson2Schema.drift.spec.ts, imports the vendored copy and reads dev_docs/cc-json-2.0.schema.json, asserting expect(vendoredSchema).toEqual(sourceSchema). This test already exists on main (confirmed via git cat-file -e main:...), is collected by the CI glob (test:ci runs .*\ +- **OVERSTATED → low** — [architecture] domainWords is first-class in the type but second-class in persistence, spawning a cascade of defensive workarounds + - `visualization/app/codeCharta/stores/fileStore/loaders/ccJson/util/fileParser.ts:17` + - Literally accurate: getExportCCFile (fileParser.ts:17-36) copies edges, markedPackages, blacklist, attributeTypes, attributeDescriptors but NOT domainWords, which is a required member of CCFile.settings.fileSettings (domain.model.ts:25). The named workarounds also exist — visibleFileStatesWithCurrentSettingsSelector (visibleFileStates.selector.ts:74) has an in-code comment describing exactly the domain-less-parse / double-commit interaction. However the severity and framing are overstated. (1) getExportCCFile emits the LEGACY 1.x ExportCCFile shape (codeCharta.api.model.ts:10), which predates +- **OVERSTATED → low** — [architecture] Two look-alike selectors with divergent memoization encode a correctness constraint in which one you import + - `visualization/app/codeCharta/stores/fileStore/store/visibleFileStates.selector.ts:74` + - The code matches the finding's technical description: visibleFileStatesSelector (line 57-59) memoizes via _onlyVisibleFilesMatterComparer, which keys only on visible-file fileChecksums (lines 20-31), so its args-comparer returns true and hands back a stale projection when file entries are replaced by richer objects under the same checksum. visibleFileStatesWithCurrentSettingsSelector (line 74) is a plain createSelector. So two look-alike selectors with divergent memoization genuinely exist. However, this is NOT an active correctness bug. The only consumer that reads per-file fileSettings domai + +## Clean Code — Grade: B + +This branch is broadly clean and internally consistent: it follows the codebase's established idioms (per-slice key->action mappers, named style/config constants, injected collaborators assembled by factories), and the sub-reviewers surfaced no correctness-threatening Clean Code defects. The one finding with real teeth is a stale migration comment that now lies about the IndexedDB migration range (says v18 while the chain and DB_VERSION reach v19) — a confirmed comment-quality defect that will actively mislead. The rest are honest, low-impact hygiene items clustered around two themes: DRY duplication of load-bearing logic (a word-ranking comparator, path-to-node-name extraction, query-string stripping) copied instead of reused, and a handful of methods/constructors that exceed the project's <25-line and 3-4-parameter rules while mirroring existing patterns. Naming and error handling are largely fine, with only isolated slips (a single-letter loop index, inline magic CSS/px strings). Nothing here blocks merge, but the duplicated comparator and stale comment are worth fixing before they drift. + +**Recurring themes:** +- Load-bearing logic duplicated instead of reusing an existing shared/exported function (comparator, path parsing, query-strip) — the copies already differ subtly and can silently drift +- Methods and constructors exceed the project's hard limits (<25 lines, max 3-4 params), mostly mirroring existing codebase idioms rather than introducing new smells +- Magic literals inline (CSS strings, px fallbacks) where the same file/branch already establishes a named-constant pattern +- Stale comments left behind by an otherwise-complete change (migration range v18 vs actual v19) + +### 🟡 MEDIUM — Migration comments claim v18 as the latest but the chain reaches v19 +`visualization/app/codeCharta/stores/rootStore/indexedDB/indexedDBWriter.ts:466` + +Confirmed by reading the file: CCSTATE_RECORD_MIGRATIONS ends at { version: 19, migrate: migrateCcStateRecordToV19 } (line 484) and DB_VERSION is 19, but the comment at line 466 still reads 'a v2 blob runs v3->...->v18; a v17 blob runs only v18' and line 506 says 'Migrate persisted blobs forward through all applicable transforms (v3->...->v18).' Both comments now understate the real migration range by one version. This is exactly the stale-comment defect the Clean Code lens flags: a future reader reasoning about the migration chain will trust an out-of-date upper bound. + +**Fix:** Update both comments (lines 466 and 506) to reference v19. Consider deriving the bound from the array rather than hardcoding it in prose so the comment cannot drift again. + +### 🟡 MEDIUM — Word-ranking comparator duplicated verbatim instead of reusing exported selectTopWords +`visualization/app/codeCharta/views/domainView/explorer/domainExplorerSelection.ts:72` + +Confirmed: domainExplorerSelection.topWords() copies the exact comparator [...words].sort((a, b) => wordSizingValue(b, sizingMode) - wordSizingValue(a, sizingMode)).slice(...) that is already implemented and EXPORTED as selectTopWords in wordCloudOption.builder.ts:122. The method's own doc comment states the intent is to rank 'the same way the cloud sizes its words', yet it duplicates the comparator rather than calling the shared function — so the tooltip ranking and the cloud ranking can silently diverge. The only real difference is the slice count (TOOLTIP_WORD_COUNT vs topN). Violates the project DRY rule for load-bearing logic. + +**Fix:** Extract a shared rankWordsBySizing(words, sizingMode) (or call selectTopWords with the tooltip count) so both the view and the renderer share one comparator definition. + +### ⚪ LOW — processFilesIndividually / SourceAnalyzer / SourceCodePipeline exceed the max 3-4 parameter rule (recurring) +`analysis/analysers/parsers/DomainLanguageParser/src/main/kotlin/de/maibornwolff/codecharta/analysers/parsers/domainlanguage/processing/FileProcessor.kt:18` + +Recurs across three Kotlin signatures on the branch. processFilesIndividually takes 5 params including two collaborator lambdas (contentReader, processor) plus an optional callback, and the same 5-param shape repeats in CoroutineFileProcessor.kt:14 and SourceAnalyzer.processFilesIndividually (SourceAnalyzer.kt:86). SourceAnalyzer's constructor injects 6 collaborators (SourceAnalyzer.kt:18) and SourceCodePipeline's constructor takes 5 (SourceCodePipeline.kt:25, mixing language/weights/ngrams/stopWordFilter/enableSsr). All exceed CLAUDE.md's 'Max 3-4 parameters'. The constructors are DI seams assembled by factories, so call-site burden is low, but the processFilesIndividually collaborator group is a genuine candidate for a small parameter object. + +**Fix:** Group the read/process/report collaborators of processFilesIndividually into one parameter object (fixes all three copies at once); optionally bundle the pipeline-configuration knobs (weights/ngrams/enableSsr) into the existing config object. Constructor DI seams are lower priority. + +### ⚪ LOW — Path-to-node-name and query-string-strip idioms re-implemented instead of reusing a single owner +`visualization/app/codeCharta/views/domainView/domainView.component.ts:71` + +Two separate DRY slips of the same shape. (1) 'last non-empty path segment' is implemented independently in domainView.component.ts:71 (`selectedNodePath()?.split("/").filter(Boolean).at(-1) ?? ""`) and wordCloud.read.store.ts:32 (`nodePath.split("/").filter(Boolean).at(-1) ?? fileRoot.rootName`); they differ only in fallback, so the collapsed explorer name and the cloud empty-state name can disagree for the same null selection. (2) redirectAwayFromDomainView.effect.ts:60 re-implements query stripping (`this.router.url.split("?")[0] === routeLinks.domain`) that routePaths.ts:33 already owns in viewIdForLink, despite routePaths.ts being documented as the routing topology 'in ONE place'. + +**Fix:** Extract a pathToNodeName(path, fallback) helper used by both view and read store; express the route check as viewIdForLink(this.router.url) === "domain" so the query-strip idiom lives only in routePaths.ts. + +### ⚪ LOW — Magic pixel and CSS literals inline where a named-constant pattern already exists (recurring) +`visualization/app/codeCharta/features/shared/components/barShell/barShell.directive.ts:13` + +Recurs across barShell.directive.ts and sidebarExplorer.component.ts: bar-height fallbacks (32px bottom bar, 17px file-extension bar, 49px combined) are hardcoded in multiple calc() strings across files (32px appears in both barShell.directive.ts:13 and sidebarExplorer.component.ts:42), so changing a default requires editing every string by hand — while BAR_GAP_PX is already a named constant right next to them. Separately, hoverTooltip.service.ts:104/110 hardcodes inline title-row/value-row CSS ('font-weight: 600; margin-bottom: 2px;', 'font-size: 10px; opacity: 0.7;') even though the same file extracts container styling into the named TOOLTIP_STYLE constant. CLAUDE.md's TS rules call out 'no magic numbers'. + +**Fix:** Centralise the bar-height fallbacks as named constants (matching BAR_GAP_PX) and extract TOOLTIP_TITLE_STYLE / TOOLTIP_ROW_STYLE to match the TOOLTIP_STYLE pattern the file already establishes. + +### ⚪ LOW — Methods exceed the 25-line rule (buildWordCloudOption, mapDomainStateToAction) +`visualization/app/codeCharta/renderer/wordCloud/util/wordCloudOption.builder.ts:156` + +Two methods pass the project's hard <25-line limit. buildWordCloudOption runs ~50 lines (156-188), most of it a single declarative echarts series[0] option literal. mapDomainStateToAction (loadInitialFile.store.ts:258) is a ~39-line 11-case dispatch switch with mechanical 'case X: dispatch(setX({ value })); break' repetition; it mirrors the existing mapMetricsLensSourceToAction/mapMapStateToAction idiom but is the longest of them. Both are low cognitive risk (declarative literal / mechanical switch) but exceed the stated bound. + +**Fix:** Extract a buildSeries(topWords, settings, context) helper from buildWordCloudOption to separate data mapping from option assembly; consider a table-driven key->action-creator map to collapse mapDomainStateToAction's cases to a lookup (would also fix the sibling mappers). + +### ⚪ LOW — detectJavaScriptFrameworks and detectCSharpFrameworks are structurally duplicated +`analysis/analysers/parsers/DomainLanguageParser/src/main/kotlin/de/maibornwolff/codecharta/analysers/parsers/domainlanguage/processing/FrameworkDetector.kt:34` + +Both methods share an identical skeleton: find files -> return emptyMap if none -> iterate -> try { extract refs; identify frameworks; store by parent dir } catch (e) { Logger.warn }. detectCSharpFrameworks (lines 66-87) mirrors detectJavaScriptFrameworks almost verbatim, and the sub-reviewer notes the two copies already diverge subtly in the merge step — the kind of drift duplication invites. Only 2 occurrences (just under the '3+ -> extract' threshold), hence low. + +**Fix:** Extract a generic helper parameterised by the file finder, dependency extractor, and framework identifier so the loop/try-catch/accumulate skeleton exists once and the merge step cannot diverge. + +### ⚪ LOW — findRedundantNgrams nests 5 levels deep (single-level-of-abstraction violation) +`analysis/analysers/parsers/DomainLanguageParser/src/main/kotlin/de/maibornwolff/codecharta/analysers/parsers/domainlanguage/processing/pipeline/stages/NgramsStage.kt:80` + +The redundancy check stacks for-loop -> for-loop -> asSequence().filter().any() -> nested any() -> boolean predicate in one method, mixing iteration control with the subsequence/frequency comparison at several abstraction levels. CLAUDE.md asks to 'split complex logic into focused helpers' and prefer guard clauses over nesting. + +**Fix:** Extract the 'is this shorter n-gram subsumed by any longer one' test into a named helper taking (shorterNgram, shorterWords, shorterFrequency) to flatten the nesting to a single level. + +### ⚪ LOW — MetricsExplorerRow spec duplicates the entire projectExplorerRow projection spec +`visualization/app/codeCharta/views/metricsView/explorer/metricsExplorerRow.spec.ts:44` + +MetricsExplorerRow.project is a thin adapter forwarding areaMetric/buildingIds/rootUnary into the pure projectExplorerRow lens, yet its spec re-verifies every branch of that lens with byte-identical fixtures and the same seven 'should ...' cases already covered by explorerRow.projection.spec.ts. The same behaviour must now be maintained in two places. + +**Fix:** Reduce the adapter spec to assert only the wiring (that injected area metric / building ids / root unary are passed through) and leave projection-logic branches to the lens spec. + +### ⚪ LOW — Single-letter loop variable 'i' in getParentDirectories +`analysis/analysers/parsers/DomainLanguageParser/src/main/kotlin/de/maibornwolff/codecharta/analysers/parsers/domainlanguage/output/DirectoryWordAggregator.kt:41` + +CLAUDE.md's 'Expressive Naming' rule requires descriptive names (>2 chars) and 'no single-letter variables'. The index counter here is named 'i'. Isolated slip on an otherwise well-named branch. + +**Fix:** Rename to depthIndex (or build the ancestor paths via a running accumulator) to comply with the naming rule. + + +## SOLID — Grade: B- + +The branch shows genuine SOLID intent — it introduced injection-token ports for the generic explorer, an ActiveViewStore abstraction, and a per-view ViewReadinessStore — but repeatedly leaves those abstractions half-applied, which is the branch's defining SOLID weakness. The clearest defect is the WordCloudComponent, which documents itself as a reusable presentational component while injecting a routing store and hard-coding the string \"domain\", making it a domain-bound container that also scrapes undocumented ECharts internals. The explorer port refactor never reached the tree-DATA path or two leaf components, so mapState leaks back in as a concretion. A recurring switch-on-type/OCP smell appears in at least four places (Framework detection, DomainState action mapping with three disagreeing lists, WordCloudSizingMode, TestFileDetector), several with silent runtime-only failure modes. None of these are architecture-breaking and the direction of travel is good, but the consistent pattern of stopping one step short of the abstraction keeps this from a B+. + +**Recurring themes:** +- Abstractions introduced but only partially applied — ports/stores exist yet high-level code still reads concretions (mapState in the explorer selector, Router.url instead of ActiveViewStore, hard-coded "domain") +- Switch-on-type/OCP: extending a concept requires coordinated edits across parallel, already-disagreeing lists, often failing only at runtime (Framework x3, DomainState x3, sizing mode, test-file naming) +- God/multi-responsibility units where a name promises one thing: FrameworkDetector, WordCloudComponent, LoadingIndicatorEffect, MetricsExplorerSelection adapter +- DIP: components/builders reaching up into global stores or newing collaborators instead of receiving projected inputs or constructor dependencies + +### 🟠 HIGH — WordCloudComponent violates its own "reusable presentational component" contract by hard-wiring the domain view +`visualization/app/codeCharta/renderer/wordCloud/components/wordCloud/wordCloud.component.ts:81` + +The class docstring (line 69) claims "the cloud stays a reusable presentational component," yet it injects the routing-layer ViewReadinessStore (line 81), hardcodes the view identity with markReady("domain") in two spots (lines 152, 191), and injects WordCloudReadStore to fetch its own data by path (line 80). A genuinely reusable renderer cannot know it is the "domain" view; this is a DIP/SRP violation where a presentational renderer reaches up into an app-global store and bakes in a caller-specific string. Reused elsewhere it would falsely mark the domain view ready. + +**Fix:** Expose a (ready) output the composing DomainViewComponent maps to markReady("domain"), and pass in data via input rather than injecting the read store, so the component is truly view-agnostic. + +### 🟡 MEDIUM — Generic explorer port refactor left the tree-data path still bound to mapState (recurs at selector and leaf level) +`visualization/app/codeCharta/features/sidebarExplorer/selectors/explorerTreeNode.selector.ts:15` + +The branch's stated goal is a generic sidebarExplorer carrying no map/domain concepts (explorerTreeLevel.component.ts:1941, write-store comment), and selectability/dimming/decoration/title/sort/context-menu were all inverted behind injection tokens. But the DIP inversion is partial in two places: (1) createExplorerTreeNodeSelector — consumed by the shared SidebarExplorerReadStore.rootNodeFor that BOTH views use — still imports areaMetricSelector from mapState directly, so the domain word-cloud view (which has no area metric) still drags in mapState as a concretion; and (2) the leaf explorerTreeItemIcon.component.ts:16 still injects SharedViewReadWindow for markedPackages$ and its sibling ExplorerTreeItemNameComponent injects SidebarExplorerReadStore for searchedNodePaths$, so the same fact (isDimmed) arrives both as a projected input and by the component reading a global store. Recurs across the subsystem: the abstraction was built but not applied to the data path. + +**Fix:** Mirror the sort port: pass areaMetric into the shared selector via a port/parameter, and funnel markedPackages/searchResult through the ExplorerRow projection so the generic components read no global slices. + +### 🟡 MEDIUM — FrameworkDetector is a god object and adding a framework requires coordinated edits across three places (SRP + OCP) +`analysis/analysers/parsers/DomainLanguageParser/src/main/kotlin/de/maibornwolff/codecharta/analysers/parsers/domainlanguage/processing/FrameworkDetector.kt:17` + +FrameworkDetector has several distinct reasons to change: walking the source tree (findPackageJsonFiles/findCsprojFiles), parsing two unrelated formats (kotlinx JSON for package.json, regex for .csproj XML), and mapping dependencies to frameworks for two ecosystems (JS/TS and C#) via duplicated discover-parse-identify-merge blocks (lines 34-54 vs 66-87). Compounding this, framework knowledge is scattered across three modules that must be edited together for one new framework: the Framework enum (FrameworkDetector.kt:10), the dependency-identification if-chains (FrameworkDetector.kt:113/122), and the framework-to-keyword-resource when in PathScopedKeywordProvider.kt:28. There is no single Framework descriptor owning its detection predicate and keyword resource, so the code is not open for extension without modification. + +**Fix:** Introduce a self-contained per-ecosystem detector and a Framework descriptor that owns its detection predicate and keyword resource, so a new framework/ecosystem is added rather than editing an exhaustive when and three parallel lists. + +### 🟡 MEDIUM — LoadingIndicatorEffect owns two responsibilities: the loading flag and cross-view readiness orchestration +`visualization/app/codeCharta/features/codeMap/effects/setLoadingIndicator/setLoadingIndicator.effect.ts:36` + +SRP: the effect's own doc (line 24) admits it "Owns two distinct things that used to be one overloaded boolean" — the isLoadingFile lifecycle (hideLoadingFileOnCommit$, line 45) and per-view readiness/staleness for ALL routed views (markViewsStaleOnDataChange$ calls viewReadinessStore.markAllStale() at line 62, invalidating the domain word-cloud view; markMetricsReadyOnRender$ at line 79 marks the metrics view ready). A codeMap-feature loading effect thus coordinates the domain view's readiness — a view it otherwise knows nothing about — and its name no longer describes what it does. + +**Fix:** Move the data-change→invalidate-all-views orchestration beside ViewReadinessStore in the routing layer, leaving this effect responsible only for the loading flag. + +### 🟡 MEDIUM — WordCloudComponent is a god component that also scrapes ECharts private internals +`visualization/app/codeCharta/renderer/wordCloud/components/wordCloud/wordCloud.component.ts:79` + +Beyond the routing coupling, the single component owns many axes of change: derived a11y text, chart init/dispose lifecycle, ResizeObserver management, a render debounce, a separate drawn-count debounce, reduced-motion detection, empty-state handling, and view-readiness signalling. The drawn-word-count concern is especially separable: countDrawnWords (line 223) casts the chart to a private EchartsWithModel shape (line 56) and walks getModel().getSeriesByIndex(0).getData().getItemGraphicEl(i) — a deep Law-of-Demeter/DIP coupling to undocumented echarts-wordcloud internals embedded directly in the view. + +**Fix:** Extract the EchartsWithModel interface, countDrawnWords, scheduleDrawnCountUpdate, drawnWordCount and droppedWordNotice into a dedicated collaborator so the component is not simultaneously a chart host and an echarts-internals scraper. + +### ⚪ LOW — Switch-on-key/type handlers that require synchronized edits across disagreeing lists to extend (recurs in three spots) +`visualization/app/codeCharta/load/loadInitialFile.store.ts:258` + +OCP: mapDomainStateToAction is an 11-branch switch over keyof DomainState whose default throws at runtime (line 294) for any unhandled key — adding a DomainState setting is not a compile error, surfacing only when a persisted blob is restored. The same knowledge is duplicated across three hand-maintained lists that already disagree: the DomainState interface (11), this switch (11), and domainStateSaveActions in actionsRequiringSaveCcState.ts (only 7 of 11 — shrinkToFit, drawOutOfBound, sortingOrder, sortingOrderAscending absent). The same anti-pattern recurs: WordCloudSizingMode is handled by a binary ternary + two hardcoded