Repository navigation
chore: folder structure ADRs - #213
Conversation
…riviere-query Three-phase design review (Architect → Refine → Critique) capturing current state analysis, separation of concerns assessment, and identified improvement opportunities. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Three-phase design review (Architect → Refine → Critique) for the TypeScript component extractor package. Also fixes markdown lint errors in existing riviere-query and eclair review documents. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Co-Authored-By: Claude Opus 4.5 <[email protected]>
📝 WalkthroughWalkthroughAdds multiple ADRs, feature-structure guidance, and extensive design-review documents across apps/packages; updates several CLAUDE.md files to reference corresponding ADRs. No executable code or public API signatures were changed. Changes
Sequence Diagram(s)(omitted — documentation-only changes; no runtime control flows introduced or modified) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
Deploying living-architecture with
|
| Latest commit: |
d45228c
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://2aca83b3.living-architecture.pages.dev |
| Branch Preview URL: | https://issue-203-architecture-revie.living-architecture.pages.dev |
ⓘ Your approaching your monthly quota for Qodo. Upgrade your plan PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
There was a problem hiding this comment.
Actionable comments posted: 20
🤖 Fix all issues with AI agents
In `@docs/architecture/adr/ADR-003-riviere-cli-architecture.md`:
- Around line 180-191: The migration table is ambiguous about where to move
validation logic; update ADR-003 to explicitly assign each source to a
destination using the decision criteria: move schema-level type definitions and
generic schema validation from component-types.ts and validation.ts into
riviere-schema, and move builder-specific validation/invariant checks and any
validation that depends on build-time context into riviere-builder; co-locate
type-specific error classes with the validation they support (schema errors →
riviere-schema, builder errors → riviere-builder) and add a single sentence
defining the boundary: "Schema validation (structural correctness) belongs in
riviere-schema; business-rule or builder invariant validation belongs in
riviere-builder."
In `@docs/architecture/adr/ADR-004-riviere-builder-architecture.md`:
- Around line 24-30: The fenced code block containing the directory structure
(the triple-backtick block showing src/, domain/, platform/, index.ts) needs a
language specifier for proper Markdown highlighting; update the opening fence
from ``` to ```text so the block becomes ```text ... ``` (i.e., add "text" after
the opening backticks) in ADR-004-riviere-builder-architecture.md.
- Around line 104-119: Update the fenced code block showing the domain directory
tree in ADR-004-riviere-builder-architecture.md to include a language specifier
(e.g., ```text) so the block is Markdown-compliant and renders with proper
syntax highlighting; locate the triple-backtick block containing the domain/
tree (the snippet with construction/, enrichment/, inspection/, error-recovery/
and filenames like graph-construction.ts, construction-types.ts,
graph-enrichment.ts, graph-inspection.ts, near-match.ts, match-types.ts) and
change the opening ``` to ```text (or another appropriate language) without
modifying the content inside the block.
In `@docs/architecture/adr/ADR-005-riviere-extract-ts-architecture.md`:
- Around line 24-29: The markdown fenced code blocks in
ADR-005-riviere-extract-ts-architecture.md lack a language tag which triggers
markdownlint; update each triple-backtick block that contains the directory
trees (the blocks showing "src/ ├── domain/ ..." at lines referenced and the
larger tree at ~35-60) to include a language identifier such as text (e.g.,
change ``` to ```text) so linting passes; ensure you update every similar fenced
block in the file (both the small top-level tree and the expanded tree sections)
to use the same convention.
In `@docs/architecture/adr/ADR-006-riviere-query-architecture.md`:
- Around line 25-30: The fenced code block in
ADR-006-riviere-query-architecture.md that shows the src/ directory tree lacks a
language specifier; update the opening fence from ``` to ```text (or another
appropriate language) so the block reads ```text and ensure the closing fence
remains ```, satisfying markdownlint and enabling proper syntax highlighting for
the directory tree.
- Around line 36-64: The fenced code block that shows the repository tree (the
block starting with the lines "src/" and listing riviere-query.ts,
component-queries.ts, etc.) is missing a language specifier; update the opening
fence from ``` to ```text (or another appropriate language) so
markdownlint/syntax highlighting recognizes it and the docs linter passes.
In `@docs/architecture/adr/ADR-007-eclair-architecture.md`:
- Around line 186-194: The ADR omits an explicit entry for the shared base error
class — add a clear line in the package structure listing the base class
(EclairError) and its file (e.g. eclair-error.ts or EclairError class) so the
architecture shows the three split error files (graph-errors, rendering-errors,
context-errors) plus the shared EclairError base; update the ADR text around
Decision 6 to include that base entry and a short note that other error classes
extend EclairError.
In `@docs/design-reviews/architecture-review-principles.md`:
- Around line 9-21: Two fenced code blocks lack language identifiers (violates
MD040); update the first tree-diagram block to use a plain text fence (e.g.,
```text) and update the second code snippet to use a TypeScript fence
(```typescript) where the sample shows "public graph: BuilderGraph" and "private
graph: BuilderGraph" so that both blocks render correctly and lint as
MD040-compliant.
In `@docs/design-reviews/eclair/critique.md`:
- Around line 17-20: The design doc claims a "PASS" for a discriminated
FlowTrace union but the code (FlowStep interface in extractFlows.ts) lacks the
discriminator; either implement the discriminated union (add a discriminant like
kind: 'EntryStepFlowTrace' | 'DownstreamFlowTrace' and replace the
depth/nullable edgeType pattern in FlowStep with EntryStepFlowTrace and
DownstreamFlowTrace types) or update refined.md to stop using "PASS" for
target-state specs (replace with "PROPOSED"/"DESIGNED", add a legend at the top
clarifying statuses, and remove any statements that assert implementation exists
for FlowTrace/EntryStepFlowTrace/DownstreamFlowTrace).
In `@docs/design-reviews/eclair/design.md`:
- Around line 254-275: Update the "Issue 1: types/riviere.ts is a Monolithic
Type File" section to reflect ADR-007 Decision 3: either (A) replace the current
split recommendation with the accepted ADR-007 action ("delete types/riviere.ts
and import types directly from riviere-schema") and remove the proposed types/
split list, or (B) keep the split recommendation only if you add an explicit
justification for why local extensions are required beyond riviere-schema
(reference "types/riviere.ts" and "ADR-007 Decision 3" in the text), so the
document no longer conflicts with the accepted ADR.
In `@docs/design-reviews/eclair/refined.md`:
- Around line 18-142: The refined layout conflicts with ADR-007: split the
single errors.ts into the ADR-defined shell/errors/ files (create
shell/errors/graph-errors.ts, rendering-errors.ts, context-errors.ts and
move/adjust the error classes referenced across the codebase), remove or stop
maintaining local riviere type files (branded-ids.ts, component-types.ts,
edge-types.ts, external-types.ts, graph-types.ts, source-location.ts) and import
those types from the canonical schema package per Decision 3 (replace local
imports in functions like computeArchitectureMetrics, GraphContext,
useRiviereQuery, etc.), and relocate the __fixtures__/ test fixtures into
platform/infra/__fixtures__/ (update any imports that reference __fixtures__/ or
riviereTestFixtures.ts). Ensure references and imports are updated project-wide
to the new paths and that ADR-007 is referenced if this design is intended to
diverge.
In `@docs/design-reviews/eclair/refinements.md`:
- Line 100: Change the phrase "domain-map specific labels" to the hyphenated
compound modifier "domain-map-specific labels" in the
docs/design-reviews/eclair/refinements.md content (the Decision line that reads
"Keep in domain-map since it only pluralizes domain-map specific labels"),
ensuring the hyphen is added between "domain-map" and "specific" to produce
"domain-map-specific labels".
In `@docs/design-reviews/riviere-builder/critique.md`:
- Line 9: The document contains a non-portable absolute path to your local
workspace (/Users/...), replace it with a repo-relative path to the same file
(for example packages/riviere-builder/src/builder.ts or
./packages/riviere-builder/src/builder.ts) so readers can locate the import of
node:fs/node:path and the save() method without relying on your machine-specific
path; update the text in critique.md to use that repo-relative path and remove
the absolute /Users/... reference.
- Line 95: Replace the unhyphenated compound modifier "600 line package" with
the hyphenated form "600-line package" in the design critique text (look for the
phrase "600 line package" in the document and the suggested-fix sentence) so the
grammar/linting noise is resolved; ensure any other occurrences of similar
numeric + noun compounds in the file follow the same hyphenation rule.
In `@docs/design-reviews/riviere-builder/refined.md`:
- Line 6: The documentation contains an absolute local path string
"/Users/nicko/code/living-architecture-issue-203-architecture-review-and-adr-fo/packages/riviere-builder/"
— replace that absolute path with a repository-relative path such as
"packages/riviere-builder/" or a relative path like
"./packages/riviere-builder/" in the
docs/design-reviews/riviere-builder/refined.md content so the doc is portable
and does not leak local filesystem information; update any other occurrences of
the same absolute path string in that file to the chosen repo-relative form.
In `@docs/design-reviews/riviere-cli/refinements.md`:
- Around line 13-19: The refinement renames CLI features to author-architecture,
extract-architecture, and explore-architecture but conflicts with ADR-003 which
uses builder, extract, and query in package/folder examples (ADR-003); either
update ADR-003 to adopt the new user-facing names (replace or add examples
showing author-architecture, extract-architecture, explore-architecture in
package/docs) or explicitly state in the refinement that the renaming is only
for user-facing surfaces (CLI/help/docs) while internal package/folder structure
remains builder/extract/query (mention ADR-003 by name and the exact internal
symbols builder, extract, query and the new user-facing symbols
author-architecture, extract-architecture, explore-architecture so readers can
reconcile them).
In `@docs/design-reviews/riviere-extract-ts/design.md`:
- Around line 5-7: The doc contains an absolute local filesystem path under
"Location:"; replace it with a repo-relative path (e.g.,
"packages/riviere-extract-ts/" or "./packages/riviere-extract-ts/") in the
design.md for the `@living-architecture/riviere-extract-ts` entry so the
documentation is portable and not tied to a specific developer machine.
In `@docs/design-reviews/riviere-extract-ts/refined.md`:
- Around line 471-482: The "Import paths changed" item is ambiguous: clarify
that package-root imports from `@living-architecture/riviere-extract-ts` remain
compatible due to the shell re-exports, but deep or relative imports will break;
update the Breaking Changes section to state that only deep/relative import
paths change, and note that root-level consumers using extractComponents ->
extractDraftComponents, evaluatePredicate -> matchesPredicate, and
evaluateFrom*Rule -> extractFrom* will be unaffected when they import from the
package root because the shell re-exports those symbols.
In `@docs/design-reviews/riviere-query/design.md`:
- Around line 236-250: The review points out a contradiction between labeling
the features/platform/shell structure as "Overkill for query-only library" and
then recommending a three-directory restructure (queries/, entity/,
branded-types/) for ~15 files; revise the recommendations section to either
strengthen justification for introducing queries/, entity/, and branded-types/
(explain clear benefits and mapping to the high-priority splits of
domain-types.ts and event-types.ts) or soften it to only mandate the two
high/medium changes (split domain-types.ts into cohesive modules and extract
Entity from event-types.ts) while moving the light folder-structure suggestion
to the low-priority note; reference domain-types.ts and event-types.ts
explicitly and remove prescriptive directory guidance unless you provide
concrete rationale for queries/, entity/, and branded-types/.
In `@docs/design-reviews/riviere-query/refined.md`:
- Around line 13-74: The proposed package layout is over‑engineered for ~15
files; collapse deeply nested dirs and keep only meaningful boundaries: move
small per-file dirs (component/, stats/, depth/, cross-domain/, external/) into
the top-level src/ alongside riviere-query.ts and index.ts, retain entity.ts and
domain-types.ts as standalone files (keep the extracted Entity class in
entity.ts and Domain/ComponentCounts interfaces in domain-types.ts), preserve
branded-types/ with its index.ts re-export for type factories, and update
imports in flow-queries.ts, event-queries.ts, graph-diff.ts, and
graph-validation.ts to the new flat paths to avoid the 1:1 file-to-directory
ratio while addressing the original high‑priority splits.
Fixes MD040 lint errors for code blocks missing language. Co-Authored-By: Claude Opus 4.5 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@docs/architecture/adr/ADR-003-riviere-cli-architecture.md`:
- Around line 118-119: Change the sentence "Entrypoints define CLI interface and
delegate to handlers:" to the tightened wording "Entrypoints define the CLI and
delegate to handlers." in the ADR document; update the text near the existing
line that currently reads "Entrypoints define CLI interface and delegate to
handlers:" to the suggested phrasing.
- ADR-003: Clarify validation destination (schema vs builder) - ADR-007: Add base eclair-error.ts to error structure Co-Authored-By: Claude Opus 4.5 <[email protected]>
Co-Authored-By: Claude Opus 4.5 <[email protected]>
|



Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.