Repository navigation
docs(design): rewrite and expand architecture design documents - #468
Conversation
Replace the old surface-level design docs with comprehensive design documents covering background, motivation, design decisions, and known limitations for each core subsystem. New documents: overview, compilation-context, command, index-design, multi-process, module, incremental, template-resolver, dependency-scanning. Removed: architecture, compilation, header-context, index.
Translate all 9 design documents from Chinese to English: overview, compilation-context, command, index-design, multi-process, module, incremental, template-resolver, dependency-scanning.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (2)
📝 WalkthroughWalkthroughDesign documentation was reorganized across English and Chinese pages. New pages cover command processing, compilation context, dependency scanning, incremental and module compilation, indexing, multi-process runtime, and template resolution; the overview pages and design sidebars were updated, and older design pages were removed or replaced. ChangesDesign documentation refresh
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🤖 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/en/design/command.md`:
- Around line 21-31: The Markdown fence in the command design doc is missing a
language tag, causing lint failures. Update the code block around the
compile_commands/json pipeline diagram to use a fenced block with the text
language tag, and keep the content unchanged; use the surrounding “command.md”
section and the diagram block itself to locate it.
In `@docs/en/design/compilation-context.md`:
- Around line 31-33: Tighten the definition of “self-contained headers” in the
compilation-context docs so `#pragma once` and include guards are not treated as
sufficient on their own. Update the wording around the “Self-contained headers”
and “Non-self-contained headers” definitions to make clear that a header is
self-contained only if it can compile independently without relying on prior
`#include` or `#define` context, using the existing “Self-contained headers”
section as the place to revise the rule.
In `@docs/en/design/incremental.md`:
- Around line 31-35: Update the preamble boundary description in the incremental
design doc so the scan does not stop at blank lines or comments before the first
preprocessor directive. In the preamble boundary section, clarify that lexical
scanning in the language server should skip leading trivia and continue until it
finds the first non-preprocessor, non-comment content, using the existing
preamble-boundary logic description.
In `@docs/en/design/index-design.md`:
- Around line 28-34: The fenced diagram in the index design docs is missing a
language tag, which triggers markdownlint. Update the diagram fence in the
documentation so it has an explicit label such as text, keeping the existing
content and structure unchanged.
- Around line 111-119: Update the OpenFileIndex/MergedIndex wording in the
design doc to make it clear that open files prefer OpenFileIndex but can fall
back to MergedIndex when the session index is dirty or unavailable. Adjust the
“mutually exclusive” phrasing near the OpenFileIndex/MergedIndex explanation so
it matches the actual behavior in resolve_cursor, and explicitly describe the
fallback path for open files instead of presenting it as a strict replacement
rule.
In `@docs/en/design/multi-process.md`:
- Around line 21-37: Add a language identifier to the fenced architecture
diagram in the multi-process design docs so the markdown lint rule is satisfied;
update the diagram block in the documentation to use a labeled fence such as
text while keeping the content unchanged.
In `@docs/en/design/overview.md`:
- Around line 169-179: The fenced diagram in the overview document is missing a
language tag, which triggers markdownlint MD040. Update the fence around the
pipeline diagram to include an explicit tag, using text if it should remain a
plain diagram, and keep the content in the same section unchanged.
In `@docs/en/design/template-resolver.md`:
- Around line 118-121: Add a language tag to the bare markdown fence in the
diagram example so it renders as plain text instead of code; update the fence
around the allocator_traits chain in the documentation snippet to use a text
fence while keeping the diagram content unchanged.
In `@docs/zh/design/command.md`:
- Around line 21-31: The fenced diagram block in the command design doc is
missing a language tag and will be flagged by markdownlint; update the code
fence in this section to use a plain text label so the renderer treats it as
text. Locate the fenced block showing the compile_commands.json flow and add the
text language marker to that fence.
In `@docs/zh/design/compilation-context.md`:
- Around line 31-33: 收紧“自包含头文件”的判定标准，避免把仅靠 `#pragma` once 或 include guard
保护的头文件误当作可独立编译的源文件；在编译上下文生成逻辑中明确要求头文件必须不依赖宿主前缀提供的前置 include
或宏定义时，才走“直接使用其编译命令”的路径。请在“自包含头文件”和“非自包含头文件”的定义处同步更新说明，确保读者和实现都按同一标准判断。
In `@docs/zh/design/incremental.md`:
- Around line 31-35: In the incremental preamble scanning logic, do not stop at
comments or blank lines when determining the preamble boundary. Update the
preamble detection described around the preamble boundary logic so it skips
leading whitespace and comments, and only ends when the first real
non-preprocessor token is encountered. Keep the behavior for identifying
`#include`, `#define`, `#pragma`, and `module;` in the same scanning path, and
ensure the zero-boundary/PCH-skip branch still only triggers when no
preprocessor content exists at all.
In `@docs/zh/design/index-design.md`:
- Around line 111-119: 这里把 OpenFileIndex 和 MergedIndex 说成“互斥”过于绝对；应改为“打开文件优先使用
OpenFileIndex，只有在 session AST 脏或不可用时由 resolve_cursor 回退到
MergedIndex”。请在这段设计说明中围绕 src/server/compiler/indexer.cpp 的 resolve_cursor
语义重写表述，明确优先级和回退路径，避免写成完全替代的关系。
- Around line 28-34: 图示的 fenced code block 缺少语言标记，导致 markdownlint
警告；请在该代码块上补一个合适的语言标记（如 text），位置就在展示 TUIndex、ProjectIndex、MergedIndex 和
OpenFileIndex 关系的那段块中，确保文档 CI 不再报错。
In `@docs/zh/design/multi-process.md`:
- Around line 21-37: The architecture diagram fenced code block in the
multi-process design document is missing a language tag, causing markdownlint to
flag it. Update that fenced block to include a valid language identifier such as
text so the diagram remains a proper markdown code block and lint passes.
In `@docs/zh/design/overview.md`:
- Around line 169-179: This fenced block is missing a language marker and
triggers markdownlint MD040; update the flowchart block in overview.md to use an
explicit plain-text language tag. Locate the fenced block near the
command/compile/semantic/index/feature diagram and add the appropriate text
label so it is treated as plain text rather than an untyped code fence.
In `@docs/zh/design/template-resolver.md`:
- Around line 118-121: In the template-resolver Markdown example, the fenced
code block is missing a language tag and is being treated as a bare fence by
markdownlint; update that snippet to use the text language marker in the fenced
block so it is explicitly marked as plain text. Locate the example containing
the allocator_traits chain and adjust the fence formatting there.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a6f6f141-b832-4e3d-a17d-d026e01467ae
📒 Files selected for processing (28)
docs/en/design/architecture.mddocs/en/design/command.mddocs/en/design/compilation-context.mddocs/en/design/compilation.mddocs/en/design/dependency-scanning.mddocs/en/design/header-context.mddocs/en/design/incremental.mddocs/en/design/index-design.mddocs/en/design/index.mddocs/en/design/module.mddocs/en/design/multi-process.mddocs/en/design/overview.mddocs/en/design/template-resolver.mddocs/en/sidebar.yamldocs/zh/design/architecture.mddocs/zh/design/command.mddocs/zh/design/compilation-context.mddocs/zh/design/compilation.mddocs/zh/design/dependency-scanning.mddocs/zh/design/header-context.mddocs/zh/design/incremental.mddocs/zh/design/index-design.mddocs/zh/design/index.mddocs/zh/design/module.mddocs/zh/design/multi-process.mddocs/zh/design/overview.mddocs/zh/design/template-resolver.mddocs/zh/sidebar.yaml
💤 Files with no reviewable changes (8)
- docs/en/design/index.md
- docs/zh/design/architecture.md
- docs/en/design/compilation.md
- docs/zh/design/compilation.md
- docs/en/design/architecture.md
- docs/zh/design/header-context.md
- docs/en/design/header-context.md
- docs/zh/design/index.md
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3307f79fa6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Add `text` language tags to bare code fences (markdownlint MD040) - Fix self-contained header definition (remove #pragma once claim) - Soften OpenFileIndex/MergedIndex from "mutually exclusive" to "preferred with fallback" - Remove unimplemented index queue dedup claim - Qualify Clang search-path consistency (note unsupported flags) - Fix stateless crash rescheduling (callers handle retry, not WorkerPool) - Fix dep-graph claim (background indexing doesn't update DependencyGraph)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bdd2f55f68
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Qualify NVCC/Intel toolchain support (generic fallback, not dedicated) - Fix currentContext to clarify it only reports explicit selections - Add session dirty-marking step to stateful crash recovery flow
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d9abd97c4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Fix eviction: remove claim about routing table clearance - Fix stateful crash: remove non-existent callback/dirty-marking steps - Fix stateless crash: tasks are lost, not retried
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d838dccd83
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| **Memory pressure feedback**: The master process periodically (every 3 seconds) checks available system memory. When available memory drops below 20% of total, `low_limit` is decremented by 1; when available memory recovers above 40%, `low_limit` is incremented by 1. This linear adjustment smoothly reduces background load under memory pressure. | ||
|
|
||
| **Crash backoff**: When a stateless worker crashes, `low_limit` is multiplied by 3/4 (multiplicative decrease). Crashes typically indicate encountering code that triggers a Clang bug; continuing at high concurrency risks more workers hitting the same problem. Multiplicative decrease is more aggressive than linear decrease, reducing system load more quickly. |
There was a problem hiding this comment.
When debugging low-memory or stateless-worker crash behavior, these thresholds/backoff won't match the implementation: Indexer::monitor_resources() adjusts indexing concurrency at <15% and >30% available memory, while WorkerPool::monitor_worker() only logs/clears stateful ownership/respawns and does not apply any stateless crash multiplier. The documented 20%/40% thresholds and 3/4 crash backoff therefore describe throttling that operators will not observe.
Useful? React with 👍 / 👎.
|
|
||
| DependencyGraph stores include relationships between files and supports both forward and reverse queries: | ||
|
|
||
| **Forward includes**: Given a file and compilation configuration, returns all files it directly includes. Each edge records whether it is a conditional include (distinguished via bit flags). A file may have different include sets under different compilation configurations (because search paths differ), so the key is a (file, configuration) pair. |
There was a problem hiding this comment.
In multi-configuration projects where the same header's nested include resolves differently under different -I sets, the scanner does not populate a (header, config) entry for every configuration: newly discovered headers are gated by scanned_files.try_emplace(path_id, ...), so the header is scanned and its includes are resolved only under the first config that discovers it. This line implies callers can ask for a file's direct includes per compilation configuration, but later configs can be missing entirely, which can mislead debugging of header-host selection in multi-config workspaces.
Useful? React with 👍 / 👎.
|
|
||
| **Discarded**: Options related to build artifacts that the language server does not need. Examples include `-o` (output file), `-c` (compilation mode), `-M` (dependency scanning), `-emit-pch` (PCH building), etc. These are simply dropped. | ||
|
|
||
| **Codegen-only**: Options that only affect the code generation backend and do not affect semantic analysis. Examples include `-fPIC`, `-fomit-frame-pointer`, `-funwind-tables`, debug info option groups (`-g*`), etc. These do not change the AST or diagnostic output and are dropped. Note that `-O` and `-fsanitize=` may appear to be code-generation-related, but they define macros (such as `__OPTIMIZE__` and `__has_feature(address_sanitizer)`), so they are retained. |
There was a problem hiding this comment.
For projects with conditional code on __PIC__, __pic__, __PIE__, or __pie__, -fPIC/-fPIE and their negative forms affect preprocessing, so they can change the AST even though this text lists -fPIC as codegen-only and says these options are dropped because they do not affect analysis. Keeping this claim will send users looking in the wrong place when clice takes a different preprocessor branch from their real build.
Useful? React with 👍 / 👎.
|
|
||
| **Step 2: Compute the include chain.** From the host source file to the target header, find the shortest path through the forward include graph. For example: `main.cpp -> utils.h -> math.h` -- if the target is `math.h`, the include chain is `[main.cpp, utils.h, math.h]`. | ||
|
|
||
| **Step 3: Synthesize the prefix code.** For each file in the include chain (except the final target file), read its contents, scan for the `#include` line that includes the next file, extract everything before that line, and add `#line` directives for accurate error location reporting. |
There was a problem hiding this comment.
For include chains where a file includes two headers with the same basename from different directories, the current prefix synthesis can stop at the wrong directive: resolve_header_context() extracts only llvm::sys::path::filename(included) and compares it to the next file's basename. This step says it scans for the line that includes the next file, but duplicate basenames can cause the synthesized prefix to omit or include the wrong code before the target header.
Useful? React with 👍 / 👎.
|
|
||
| Project-level global state -- the single source of truth from disk. | ||
|
|
||
| - **Workspace**: Holds the compilation database, toolchain, path pool, dependency graph, PCH/PCM cache, project index, and all other project-level state. Core invariant: unsaved buffer contents of open files never modify the Workspace. Workspace state changes come from three paths: initial load, cascading updates triggered by file saves (didSave), and index merges after background indexing completes. |
There was a problem hiding this comment.
This invariant is too strong for normal unsaved editing: Compiler::ensure_pch() builds from session.text, then writes the result into workspace.pch_cache and persists cache.json, and PCM preparation also updates Workspace caches during demand-driven requests. Developers relying on this description will miss cache mutations that happen without didSave or index merges.
Useful? React with 👍 / 👎.
1 participant
Summary
Test plan
pixi run formatSummary by CodeRabbit