chore(docs): record architecture taxonomy decisions - #207
Conversation
yordis
commented
Jun 8, 2026
- Preserve stable architecture vocabulary before future package and runtime moves create inconsistent boundaries.
- Reduce ambiguity around services, crates, transports, and SDK names during upcoming implementation work.
PR SummaryLow Risk Overview Docs site: VitePress config becomes async and loads ADRs via new Repo policy: Reviewed by Cursor Bugbot for commit 0b6bfcb. Bugbot is set up for automated code reviews on this repo. Configure here. |
65c096a to
ddf36eb
Compare
|
Warning Review limit reached
More reviews will be available in 2 minutes and 23 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (15)
WalkthroughEstablishes ADR infrastructure for documenting architectural decisions: VitePress integration with dynamic sidebar generation, five foundational ADRs addressing workspace organization and design patterns, and governance rules for the experiments directory enabling speculative work. ChangesArchitecture Decision Records System
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
ddf36eb to
c2f5fbc
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/.vitepress/config.mts`:
- Line 7: Replace the URL-to-path usage when calling readAdrRecords: instead of
passing new URL("..", import.meta.url).pathname to readAdrRecords (which can
produce percent-encoded or non-native paths), import and use fileURLToPath to
convert import.meta.url to a real filesystem path and pass that into
readAdrRecords; update the call site where const adrRecords = await
readAdrRecords(...) is invoked to use fileURLToPath(new URL("..",
import.meta.url)) so readAdrRecords receives a proper platform-native path.
In `@scripts/start_adr.sh`:
- Around line 15-18: After normalizing title into the slug variable in
start_adr.sh (the slug="$(printf '%s' "$title" | tr ... | sed -E
's/[^a-z0-9]+/-/g; s/^-+//; s/-+$//')" line), add a validation step that checks
whether slug is non-empty; if empty, either exit with a clear error message
(using echo or logger and non-zero exit) or generate a safe fallback (e.g.,
"untitled" + timestamp) before proceeding to write the ADR filename—ensure this
check runs after the sed normalization and is applied similarly where slug is
created again at the other location mentioned.
- Around line 19-23: The script attempts to compute last_number by running find
on adr_dir before ensuring the directory exists, which causes failures on first
run; update the startup sequence to create the ADR directory (use mkdir -p on
the variable adr_dir) before any use of find or other scans (affecting the
last_number assignment and the similar block around line 33), so ensure adr_dir
is created prior to running the find "$adr_dir" ... pipeline that sets
last_number.
- Line 36: The Perl one-liner interpolates raw $slug/$title into s/// which
treats characters like & specially; change the call to pass slug/title as
environment variables and use Perl's \Q...\E (quotemeta) in the replacement so
values are treated literally. Concretely, set TITLE="$title" SLUG="$slug" before
invoking perl and replace the inline s/.../$slug/g and s/.../$title/g with
s/<!-- ADR title -->/\Q$ENV{TITLE}\E/g and s/<!-- ADR slug -->/\Q$ENV{SLUG}\E/g
(keep the other substitutions for ADR number/padded/date as-is).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 62f1910f-3207-41a1-aa24-bf268b3b247d
⛔ Files ignored due to path filters (1)
docs/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
docs/.vitepress/config.mtsdocs/.vitepress/helpers.tsdocs/adr/0001-workspace-runtime-taxonomy.mddocs/adr/0002-rust-crate-boundaries.mddocs/adr/0003-ai-protocol-transport-taxonomy.mddocs/adr/0004-protocol-and-transport-layering.mddocs/adr/0005-polyglot-workspace-layout.mddocs/adr/index.mddocs/package.jsonscripts/start_adr.shtemplates/adr.md
Code Coverage SummaryDetailsDiff against mainResults for commit: 032bdab Minimum allowed coverage is ♻️ This comment has been updated with latest results |
032bdab to
85f4a46
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 85f4a46. Configure here.
85f4a46 to
414c8a4
Compare
Establish workspace architecture ADRs covering taxonomy, polyglot boundaries, record discovery, wire contracts, and internal contract governance to reduce ambiguity before package and runtime moves. Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
414c8a4 to
0b6bfcb
Compare
Brings in #207 (ADR taxonomy docs), #208 (derive error implementations via thiserror), #210 (testcontainers ADR). Conflict resolutions (converge to main's thiserror direction, keep platform functionality): - connect.rs: thiserror derive + keep AuthorizationViolation variant and platform's richer connect logic; MAX_RECONNECT_DELAY from crate::constants - lease_config_error.rs: thiserror derive, keep platform tests - verification_token.rs: thiserror derive; keep #[allow(dead_code)] on latest (lib/bin split makes it dead in the lib crate) - provision.rs: typed ProvisionError { source: Box<dyn Error> }, keep platform's create-and-update behavior Collateral fixes: - trogon-wasm-runtime: anyhow pin =1.0.98 -> workspace (=1.0.102) - acp-nats config.rs tests: AcpPrefixError is now an enum, match the variant Verified: cargo build --workspace, all workspace tests compile, affected crate tests pass, cargo clippy --workspace clean.
Design/implementation plan for selecting any model from any runner provider (acp/xai/openrouter) as the context-compaction model in CLI and IDE, defaulting to the session model. Covers the catalog keystone (S1), wire/selection/reactive strategy (M1–M4), and the architecture-ADR conventions (PR #207).
Allow selecting any model from any provider (acp/xai/openrouter, of which a credential exists) as the context-compaction model, in CLI and IDE, defaulting to the session model. - S1: NATS-backed model catalog (trogonai-catalog service + trogonai-catalog-client with the shared pure predicate [type+capacity+credential filters] and the provider-qualified codec); proxy provider introspection (trogon.proxy.providers). - M1: protobuf compactor wire (compactor_provider, fallback_model); routes by provider. - M2: acp-runner participates (compactor_provider in build_compact_payload + 2 call sites). - M3: provider-qualified selection from the shared predicate (CLI /compact-model with autocomplete, IDE dropdown); Gap C resolved via runner config + catalog/codec (registry untouched). - M3b: embedded trogon-acp builds the cross-provider option from the cached catalog. - M4: reactive fallback to the session model on any compaction error. - Persistence (ADR 0009): ACP session as protobuf SessionRecord; xai/openrouter compaction override as a separate protobuf CompactionConfig record (the console- shared SESSIONS JSON bucket is left untouched, ADR 0009 interop exception). Contracts/telemetry/config/naming follow the architecture ADRs (PR #207): protobuf wire & persistence (.proto under proto/, *-proto crates), with the ACP-owned surfaces kept in ACP format.
Brings in #207 (ADR taxonomy docs), #208 (derive error implementations via thiserror), #210 (testcontainers ADR). Conflict resolutions (converge to main's thiserror direction, keep platform functionality): - connect.rs: thiserror derive + keep AuthorizationViolation variant and platform's richer connect logic; MAX_RECONNECT_DELAY from crate::constants - lease_config_error.rs: thiserror derive, keep platform tests - verification_token.rs: thiserror derive; keep #[allow(dead_code)] on latest (lib/bin split makes it dead in the lib crate) - provision.rs: typed ProvisionError { source: Box<dyn Error> }, keep platform's create-and-update behavior Collateral fixes: - trogon-wasm-runtime: anyhow pin =1.0.98 -> workspace (=1.0.102) - acp-nats config.rs tests: AcpPrefixError is now an enum, match the variant Verified: cargo build --workspace, all workspace tests compile, affected crate tests pass, cargo clippy --workspace clean. Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Capture the architecture-taxonomy ADRs (PR #207) against the codex parity work: Phase 0.5 passes ADR 0004 (protocol/transport layering) and the codex JSON-RPC wire is an explicit ADR 0009 exception. Crate naming / env-var prefix / tracing-vs-OTel are pre-existing workspace-wide deviations (renaming out of scope). Flags the one forward action: Phase 5/6 KV persistence must use a versioned protobuf contract or carry the documented ADR 0009 migrate-when-touched exception. Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Allow selecting any model from any provider (acp/xai/openrouter, of which a credential exists) as the context-compaction model, in CLI and IDE, defaulting to the session model. - S1: NATS-backed model catalog (trogonai-catalog service + trogonai-catalog-client with the shared pure predicate [type+capacity+credential filters] and the provider-qualified codec); proxy provider introspection (trogon.proxy.providers). - M1: protobuf compactor wire (compactor_provider, fallback_model); routes by provider. - M2: acp-runner participates (compactor_provider in build_compact_payload + 2 call sites). - M3: provider-qualified selection from the shared predicate (CLI /compact-model with autocomplete, IDE dropdown); Gap C resolved via runner config + catalog/codec (registry untouched). - M3b: embedded trogon-acp builds the cross-provider option from the cached catalog. - M4: reactive fallback to the session model on any compaction error. - Persistence (ADR 0009): ACP session as protobuf SessionRecord; xai/openrouter compaction override as a separate protobuf CompactionConfig record (the console- shared SESSIONS JSON bucket is left untouched, ADR 0009 interop exception). Contracts/telemetry/config/naming follow the architecture ADRs (PR #207): protobuf wire & persistence (.proto under proto/, *-proto crates), with the ACP-owned surfaces kept in ACP format. Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Design/implementation plan for selecting any model from any runner provider (acp/xai/openrouter) as the context-compaction model in CLI and IDE, defaulting to the session model. Covers the catalog keystone (S1), wire/selection/reactive strategy (M1–M4), and the architecture-ADR conventions (PR #207). Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
