Repository navigation
Reduce cognitive complexity across dash_discover, tool, and scripts - #59
Merged
Merged
Conversation
- Flatten neighbor and declaration checks in `path_package_rule.dart`, `outline_generator.dart`, and `package_facts.dart` into the `<= 15` target zone without introducing shallow helpers. - Extract pure file-private helpers in `cli.dart`, `skills_catalog.dart`, `check_aux_effect.dart`, `tool/bin/readme.dart`, and `profile.dart`, reducing max cognitive complexity from 64 to 15 (net delta -59) with zero public API changes.
- profile.dart: bind the decoded line stream to a local so
`_listenForServiceUri` uses `.listen((line) {` without the formatter's
awkward parameter wrap (a single-return block body was reflowed back
and tripped prefer_expression_function_bodies).
- outline_generator.dart: split `++lineCount` from the max-lines check.
- package_facts.dart: restore the plain `for` loop over duplicates; walk
lib then aux declarations lazily via `followedBy` instead of a
materialized list (restoring the `addEdges` closure scores CC 17).
kevmoo
added a commit
to kevmoo/analytica.dart
that referenced
this pull request
Oct 9, 2026
…184) The `dart-cognitive-complexity` skill pushes agents to clear every `shallow` and `file_split` finding, which caused over-refactoring and regressions in dogfood runs. ## Changes - **Stop rule:** only declarations above the threshold are work items. When max CC <= threshold, report done and stop; anything further needs a user ask. - **Advisory, not gates:** `shallow` and `file_split` are documented as optional review aids. Removed the "batch re-inline SAFE_INLINE" triage option and every `--fail-on-safe-inline` gate recommendation. The findings-table action now reads "Review: consider re-inlining if ...". - **Keep/revert heuristics (new §5.3):** keep de-duplication, guard clauses, and named step helpers. Don't inline helpers with same-shape siblings or meaningful names/docs, don't make style-only edits, and don't split files without a cohesive name (no barrels or cycles). - **Hot-loop rule (new §5.4):** benchmark before splitting per-element kernels. Prefer an inline kernel with `// cognitive_complexity:ignore` on the line before the declaration, plus a reason comment. - **Blind self-review:** for diffs over ~300 lines, a fresh agent or human reviewer given only the before/after trees and diff judges each change. - **`refactoring_recipes.md`:** Pattern G no longer says "always re-inline". Pattern F now says to name split files by their contents. - **Evals:** replaced the shallow-gate assertion and added negative assertions (no batch re-inline, sibling helpers stay extracted, no style-only edits). Added `complexity_stop_when_clean`. Docs-only: no version, pin, or CHANGELOG changes. ## Evidence Blind reviews of the dogfood PRs found that fixing CC > 15 helped, but re-inlining `SAFE_INLINE` helpers and name-by-one-function file splits mostly hurt: - kevmoo/qr.dart#174, kevmoo/dash_skills#59, kevmoo/pubviz#196, kevmoo/scripts.dart#147 - kevmoo/qr.dart#175 restored an inline hot kernel after the helper split made generation 39-66% slower on AOT Related (not fixed here): #181, #183
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reduces every function in
packages/dash_discover,tool/bin, andskills/profile-dart-code/scriptsto cognitive complexity <= 15 (max was 64). No public API changes.profile.dart: merge the duplicated stdout/stderr VM-service listeners into_listenForServiceUri;_waitForPauseAtExitreturns directly instead of a mutable flag +breaks; hoist the URI regex.cli.dart:runClibecomes a short dispatcher;_handleSkillDocSyncuses one explicit branch;_discoverCatalogremoves a duplicated discovery call.skills_catalog.dart: default search-path logic moves to_resolveSearchPaths.check_aux_effect.dart:_findRejectedByAuxreturns the details list, dropping a redundant counter.outline_generator.dart,path_package_rule.dart,package_facts.dart: flatten neighbor/declaration checks (package_factsiterates lib then aux declarations viafollowedBy, no intermediate list).tool/bin/readme.dart: table rendering moves to_buildSkillsTable.Gates:
cognitive_complexity --fail-threshold 15,dart format,dart analyze --fatal-infos, anddart testin all three packages;kscripts pr-check.