Open
Conversation
🤖 Kimi Code ReviewThis is a documentation-only PR adding an interactive HTML architecture diagram. No Rust code changes, so no security or correctness issues to review. The diagram accurately reflects the current actor-based design with:
The documentation is well-structured and will help new contributors understand the codebase architecture. The interactive elements (clickable nodes, tabs) work as expected. Automated review by Kimi (Moonshot AI) · custom prompt |
🤖 Codex Code Review
Assumptions:
Overall:
Automated review by OpenAI Codex · custom prompt |
🤖 Claude Code ReviewNow I have a complete picture of the PR. Let me compile the review. PR Review: docs — Interactive Architecture DiagramThis PR adds a single-file interactive HTML architecture diagram ( External Font Dependency (Functional Concern)The file loads fonts from Google Fonts at runtime: <link href="https://fonts.googleapis.com/css2?family=Inter:wght@300;400;500;600;700&family=JetBrains+Mono:wght@400;500&display=swap" rel="stylesheet">
Consider either inlining the font via Tick Interval Count DiscrepancyThe '800ms', '5 intervals', 'send_after', '#[protocol]'
// ...
'Interval 3/4: accept accumulated attestations',The flow card also reads "Interval 3/4: accept atts". However, If the code genuinely has 5 intervals at 800ms, Minor Content Notes
What's Working Well
SummaryNo blocking issues. The font dependency is the most practical concern for usability. The tick interval discrepancy is worth clarifying against the source code before merging to avoid documentation drift. Automated review by Claude (Anthropic) · custom prompt |
Contributor
Greptile SummaryThis PR adds a single self-contained interactive HTML file ( Key findings:
Confidence Score: 4/5
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
Main["bin/ethlambda\n(tokio::main)"]
Main -->|"InitP2P / InitBlockChain"| BC
Main -->|spawn| P2P
Main -->|start_rpc_server| RPC
subgraph Consensus ["BlockChain Actor (crates/blockchain)"]
BC["BlockChain\n#[actor]"]
Store["Store\n(fork choice state)"]
KeyMgr["KeyManager\n(XMSS signing)"]
ForkChoice["LMD GHOST\n(fork_choice)"]
STF["State Transition\n(process_slots / process_block)"]
BC --> Store
BC --> KeyMgr
Store --> ForkChoice
Store --> STF
end
subgraph P2PLayer ["P2P Actor (crates/net)"]
P2P["P2PServer\n#[actor]"]
SwarmAdapter["SwarmAdapter\n(tokio task)"]
GossipSub["GossipSub\n(blocks + attestations)"]
ReqResp["Req/Resp\n(Status + BlocksByRoot)"]
P2P --> SwarmAdapter
SwarmAdapter --> GossipSub
SwarmAdapter --> ReqResp
end
subgraph Shared ["Shared Infrastructure"]
RocksDB["RocksDB Backend\nArc<dyn StorageBackend>"]
Crypto["XMSS Crypto\n(leansig + leanVM)"]
end
NetworkAPI["network-api\n(Recipient<M> messages)"]
BC <-->|"Recipient<NewBlock>\nRecipient<PublishBlock>"| NetworkAPI
NetworkAPI <-->|"typed actor messages"| P2P
RPC -->|"read Store directly"| RocksDB
BC -->|"persist"| RocksDB
P2P -->|"persist"| RocksDB
KeyMgr --> Crypto
Last reviewed commit: 1cef6e8 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+966
to
+974
| let html = '<div class="section-label">Overview</div><p>' + data.desc + '</p>'; | ||
| if (data.tags?.length) { | ||
| html += '<div class="section-label">Tags</div>'; | ||
| html += data.tags.map(t => '<span class="tag" style="background:' + lc.bg + ';border:1px solid ' + lc.border + ';color:' + lc.text + ';">' + t + '</span>').join(''); | ||
| } | ||
| if (data.details?.length) { | ||
| html += '<div class="section-label">Details</div><ul>' + data.details.map(d => '<li>' + d + '</li>').join('') + '</ul>'; | ||
| } | ||
| body.innerHTML = html; |
Contributor
There was a problem hiding this comment.
Type parameters stripped from rendered detail panel
showDetail() builds the HTML for tags and details by directly concatenating raw strings from the COMPONENTS object into an innerHTML assignment. Several tags and details values contain angle-bracket notation (e.g. Handler<M>, Handler<InitP2P>, Recipient<M>, start_swarm_adapter() -> (Stream<SwarmEvent>, SwarmHandle)) that the browser will parse as unknown HTML elements rather than literal text.
For single-letter type params like <M>, the browser creates an empty unknown element and the letter is swallowed — so Handler<M> renders as just "Handler". For longer type params like <InitP2P>, the element name disappears and the following text becomes the hidden inner text of an unknown element.
Affected values include (non-exhaustive):
- Tags:
Handler<M>(blockchain, p2p),Recipient<M>(netapi) - Details:
Handler<InitP2P>,Handler<InitBlockChain>,Handler<PublishBlock>,Handler<NewBlock>,Handler<WrappedSwarmEvent>,Recipient<M> is type-erased…,start_swarm_adapter() -> (Stream<SwarmEvent>…)
Fix by escaping < and > before inserting into HTML:
Suggested change
| let html = '<div class="section-label">Overview</div><p>' + data.desc + '</p>'; | |
| if (data.tags?.length) { | |
| html += '<div class="section-label">Tags</div>'; | |
| html += data.tags.map(t => '<span class="tag" style="background:' + lc.bg + ';border:1px solid ' + lc.border + ';color:' + lc.text + ';">' + t + '</span>').join(''); | |
| } | |
| if (data.details?.length) { | |
| html += '<div class="section-label">Details</div><ul>' + data.details.map(d => '<li>' + d + '</li>').join('') + '</ul>'; | |
| } | |
| body.innerHTML = html; | |
| function escapeHtml(str) { | |
| return str.replace(/&/g, '&').replace(/</g, '<').replace(/>/g, '>'); | |
| } | |
| function showDetail(id) { | |
| const data = COMPONENTS[id]; | |
| if (!data) return; | |
| const langColors = { | |
| consensus: { bg: 'rgba(155,89,182,0.15)', border: 'var(--consensus)', text: 'var(--consensus-l)' }, | |
| net: { bg: 'rgba(0,180,216,0.15)', border: 'var(--net)', text: 'var(--net-l)' }, | |
| storage: { bg: 'rgba(46,204,113,0.15)', border: 'var(--storage)', text: 'var(--storage-l)' }, | |
| crypto: { bg: 'rgba(243,156,18,0.15)', border: 'var(--crypto)', text: 'var(--crypto-l)' }, | |
| types: { bg: 'rgba(231,76,60,0.15)', border: 'var(--types)', text: 'var(--types-l)' }, | |
| entry: { bg: 'rgba(224,123,57,0.2)', border: 'var(--rust)', text: 'var(--rust-light)' }, | |
| api: { bg: 'rgba(26,188,156,0.15)', border: 'var(--api)', text: 'var(--api-l)' }, | |
| }; | |
| const lc = langColors[data.lang] || langColors.entry; | |
| const header = document.getElementById('panel-header'); | |
| const body = document.getElementById('panel-body'); | |
| const ph = document.getElementById('panel-placeholder'); | |
| header.style.display = 'block'; | |
| header.style.borderColor = lc.border; | |
| document.getElementById('panel-title').textContent = data.title; | |
| document.getElementById('panel-title').style.color = lc.text; | |
| document.getElementById('panel-subtitle').textContent = data.subtitle; | |
| if (ph) ph.remove(); | |
| let html = '<div class="section-label">Overview</div><p>' + escapeHtml(data.desc) + '</p>'; | |
| if (data.tags?.length) { | |
| html += '<div class="section-label">Tags</div>'; | |
| html += data.tags.map(t => '<span class="tag" style="background:' + lc.bg + ';border:1px solid ' + lc.border + ';color:' + lc.text + ';">' + escapeHtml(t) + '</span>').join(''); | |
| } | |
| if (data.details?.length) { | |
| html += '<div class="section-label">Details</div><ul>' + data.details.map(d => '<li>' + escapeHtml(d) + '</li>').join('') + '</ul>'; | |
| } | |
| body.innerHTML = html; | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: docs/infographics/ethlambda_architecture.html
Line: 966-974
Comment:
**Type parameters stripped from rendered detail panel**
`showDetail()` builds the HTML for tags and details by directly concatenating raw strings from the `COMPONENTS` object into an `innerHTML` assignment. Several `tags` and `details` values contain angle-bracket notation (e.g. `Handler<M>`, `Handler<InitP2P>`, `Recipient<M>`, `start_swarm_adapter() -> (Stream<SwarmEvent>, SwarmHandle)`) that the browser will parse as unknown HTML elements rather than literal text.
For single-letter type params like `<M>`, the browser creates an empty unknown element and the letter is swallowed — so `Handler<M>` renders as just **"Handler"**. For longer type params like `<InitP2P>`, the element name disappears and the following text becomes the hidden inner text of an unknown element.
Affected values include (non-exhaustive):
- Tags: `Handler<M>` (blockchain, p2p), `Recipient<M>` (netapi)
- Details: `Handler<InitP2P>`, `Handler<InitBlockChain>`, `Handler<PublishBlock>`, `Handler<NewBlock>`, `Handler<WrappedSwarmEvent>`, `Recipient<M> is type-erased…`, `start_swarm_adapter() -> (Stream<SwarmEvent>…)`
Fix by escaping `<` and `>` before inserting into HTML:
```suggestion
function escapeHtml(str) {
return str.replace(/&/g, '&').replace(/</g, '<').replace(/>/g, '>');
}
function showDetail(id) {
const data = COMPONENTS[id];
if (!data) return;
const langColors = {
consensus: { bg: 'rgba(155,89,182,0.15)', border: 'var(--consensus)', text: 'var(--consensus-l)' },
net: { bg: 'rgba(0,180,216,0.15)', border: 'var(--net)', text: 'var(--net-l)' },
storage: { bg: 'rgba(46,204,113,0.15)', border: 'var(--storage)', text: 'var(--storage-l)' },
crypto: { bg: 'rgba(243,156,18,0.15)', border: 'var(--crypto)', text: 'var(--crypto-l)' },
types: { bg: 'rgba(231,76,60,0.15)', border: 'var(--types)', text: 'var(--types-l)' },
entry: { bg: 'rgba(224,123,57,0.2)', border: 'var(--rust)', text: 'var(--rust-light)' },
api: { bg: 'rgba(26,188,156,0.15)', border: 'var(--api)', text: 'var(--api-l)' },
};
const lc = langColors[data.lang] || langColors.entry;
const header = document.getElementById('panel-header');
const body = document.getElementById('panel-body');
const ph = document.getElementById('panel-placeholder');
header.style.display = 'block';
header.style.borderColor = lc.border;
document.getElementById('panel-title').textContent = data.title;
document.getElementById('panel-title').style.color = lc.text;
document.getElementById('panel-subtitle').textContent = data.subtitle;
if (ph) ph.remove();
let html = '<div class="section-label">Overview</div><p>' + escapeHtml(data.desc) + '</p>';
if (data.tags?.length) {
html += '<div class="section-label">Tags</div>';
html += data.tags.map(t => '<span class="tag" style="background:' + lc.bg + ';border:1px solid ' + lc.border + ';color:' + lc.text + ';">' + escapeHtml(t) + '</span>').join('');
}
if (data.details?.length) {
html += '<div class="section-label">Details</div><ul>' + data.details.map(d => '<li>' + escapeHtml(d) + '</li>').join('') + '</ul>';
}
body.innerHTML = html;
}
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+1033
to
+1037
| const bc = getCenter(document.getElementById('an-blockchain'), canvas); | ||
| const p2p = getCenter(document.getElementById('an-p2p'), canvas); | ||
| const api = getCenter(document.getElementById('an-netapi'), canvas); | ||
| makeArrow(svg, bc.right, bc.cy - 6, p2p.left, p2p.cy - 6, 'rgba(26,188,156,0.7)', 'arr-api', true); | ||
| makeArrow(svg, p2p.left, p2p.cy + 6, bc.right, bc.cy + 6, 'rgba(26,188,156,0.7)', 'arr-api', true); |
Contributor
There was a problem hiding this comment.
Unused variable api
api is computed via getCenter() on line 1035 but never referenced. Both makeArrow calls on the following lines use only bc and p2p. Remove the dead code:
Suggested change
| const bc = getCenter(document.getElementById('an-blockchain'), canvas); | |
| const p2p = getCenter(document.getElementById('an-p2p'), canvas); | |
| const api = getCenter(document.getElementById('an-netapi'), canvas); | |
| makeArrow(svg, bc.right, bc.cy - 6, p2p.left, p2p.cy - 6, 'rgba(26,188,156,0.7)', 'arr-api', true); | |
| makeArrow(svg, p2p.left, p2p.cy + 6, bc.right, bc.cy + 6, 'rgba(26,188,156,0.7)', 'arr-api', true); | |
| const bc = getCenter(document.getElementById('an-blockchain'), canvas); | |
| const p2p = getCenter(document.getElementById('an-p2p'), canvas); | |
| makeArrow(svg, bc.right, bc.cy - 6, p2p.left, p2p.cy - 6, 'rgba(26,188,156,0.7)', 'arr-api', true); | |
| makeArrow(svg, p2p.left, p2p.cy + 6, bc.right, bc.cy + 6, 'rgba(26,188,156,0.7)', 'arr-api', true); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: docs/infographics/ethlambda_architecture.html
Line: 1033-1037
Comment:
**Unused variable `api`**
`api` is computed via `getCenter()` on line 1035 but never referenced. Both `makeArrow` calls on the following lines use only `bc` and `p2p`. Remove the dead code:
```suggestion
const bc = getCenter(document.getElementById('an-blockchain'), canvas);
const p2p = getCenter(document.getElementById('an-p2p'), canvas);
makeArrow(svg, bc.right, bc.cy - 6, p2p.left, p2p.cy - 6, 'rgba(26,188,156,0.7)', 'arr-api', true);
makeArrow(svg, p2p.left, p2p.cy + 6, bc.right, bc.cy + 6, 'rgba(26,188,156,0.7)', 'arr-api', true);
```
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+52
to
+58
| html, body { | ||
| height: 100%; | ||
| font-family: 'Inter', sans-serif; | ||
| background: var(--bg); | ||
| color: var(--text); | ||
| overflow: hidden; | ||
| } |
Contributor
There was a problem hiding this comment.
External font dependency — missing offline fallback
The diagram loads Inter and JetBrains Mono exclusively from fonts.googleapis.com (line 7). The CSS font stacks provide no system-font fallbacks, so the diagram will not render with intended typography when viewed offline, and each viewer's browser makes a request to Google's CDN.
Consider adding system-font fallbacks so the file remains usable without internet access:
Suggested change
| html, body { | |
| height: 100%; | |
| font-family: 'Inter', sans-serif; | |
| background: var(--bg); | |
| color: var(--text); | |
| overflow: hidden; | |
| } | |
| html, body { | |
| height: 100%; | |
| font-family: 'Inter', system-ui, -apple-system, sans-serif; | |
| background: var(--bg); | |
| color: var(--text); | |
| overflow: hidden; | |
| } |
Also update monospace font usage (lines 79, 109, 120, 124, etc.) from 'JetBrains Mono', monospace to include fallbacks like 'JetBrains Mono', 'Fira Code', 'Cascadia Code', monospace.
Prompt To Fix With AI
This is a comment left during a code review.
Path: docs/infographics/ethlambda_architecture.html
Line: 52-58
Comment:
**External font dependency — missing offline fallback**
The diagram loads `Inter` and `JetBrains Mono` exclusively from `fonts.googleapis.com` (line 7). The CSS font stacks provide no system-font fallbacks, so the diagram will not render with intended typography when viewed offline, and each viewer's browser makes a request to Google's CDN.
Consider adding system-font fallbacks so the file remains usable without internet access:
```suggestion
html, body {
height: 100%;
font-family: 'Inter', system-ui, -apple-system, sans-serif;
background: var(--bg);
color: var(--text);
overflow: hidden;
}
```
Also update monospace font usage (lines 79, 109, 120, 124, etc.) from `'JetBrains Mono', monospace` to include fallbacks like `'JetBrains Mono', 'Fira Code', 'Cascadia Code', monospace`.
How can I resolve this? If you propose a fix, please make it concise.
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.
This PR adds an interactive architecture diagram for ethlambda.
To visualize it, run: