Open
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new /api/height fast-poll endpoint, enriches /api/status with chain metadata, fetches chain_id from RPC at startup and wires CHAIN_NAME config into AppState, generalizes table-counting, and adds a frontend StatusPage and route that polls height every 5s. Changes
Sequence Diagram(s)sequenceDiagram
participant Startup as Application Startup
participant RPC as RPC Provider
participant DB as Database
participant API as Atlas API
participant Client as Browser StatusPage
participant State as AppState
Startup->>RPC: eth_chainId (fetch_chain_id)
RPC-->>Startup: "0x..." (hex)
Startup->>Startup: parse_chain_id(hex) -> chain_id
Startup->>State: init AppState { chain_id, chain_name, ... }
Startup->>DB: connect / migrate
Client->>API: GET /api/status (getChainStatus)
API->>State: read chain_id, chain_name
API->>DB: query totals, block_height, indexed_at
API-->>Client: ChainStatusResponse
loop every 5s
Client->>API: GET /api/height
API->>DB: read latest block_height, indexed_at
API-->>Client: HeightResponse
Client->>Client: update UI
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
pthmas
force-pushed
the
pierrick/status-page
branch
from
6fcae3a to
8df58c9
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/crates/atlas-api/src/handlers/status.rs (1)
27-63:⚠️ Potential issue | 🟠 MajorAdd same-file unit tests for the new status handler logic.
This file adds new response shapes and query flow but does not include a
#[cfg(test)] mod testsblock here.As per coding guidelines "Add unit tests for new logic in a
#[cfg(test)] mod testsblock in the same file. Run all tests withcargo test --workspace."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/crates/atlas-api/src/handlers/status.rs` around lines 27 - 63, Add a #[cfg(test)] mod tests block to this file containing unit tests for get_height and get_status: mock or construct an Arc<AppState> with a test sqlx::Sqlite/Postgres in-memory pool (or use sqlx::query mocking), insert a row for key 'last_indexed_block', and assert that get_height returns the expected HeightResponse and get_status returns ChainStatus with correct block_height, indexed_at, total_transactions and total_addresses (exercise get_table_count by creating test rows for "transactions" and "addresses" or mocking it). Name tests clearly (e.g., test_get_height_returns_parsed_height, test_get_status_returns_full_chain_status), import necessary types (AppState, HeightResponse, ChainStatus, get_height, get_status, get_table_count) and run them with cargo test --workspace to ensure compilation and behavior.
🧹 Nitpick comments (1)
backend/crates/atlas-api/src/handlers/mod.rs (1)
55-58: Consider identifier quoting for defense-in-depth.The
format!macro directly interpolatestable_nameinto the SQL query. While all current callers pass hardcoded string literals (verified instatus.rsandtransactions.rs), adding identifier quoting would provide defense-in-depth against future misuse.Optional: Quote the identifier
// Exact count for small tables - let exact: (i64,) = sqlx::query_as(&format!("SELECT COUNT(*) FROM {table_name}")) + let exact: (i64,) = sqlx::query_as(&format!("SELECT COUNT(*) FROM \"{}\"", table_name)) .fetch_one(pool) .await?;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/crates/atlas-api/src/handlers/mod.rs` around lines 55 - 58, The query interpolates table_name directly into sql and should quote and escape the identifier for defense-in-depth: when building the sql for sqlx::query_as (where variable table_name is used to produce exact), wrap the identifier in double quotes and escape any embedded double quotes by replacing " with "" before interpolation (or use sqlx's QueryBuilder/identifier-quoting helper if available) so the final string becomes a safely quoted table identifier instead of raw table_name.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.env.example:
- Around line 4-6: The CHAIN_NAME value contains spaces and should be quoted;
update the .env.example entry for CHAIN_NAME to surround the value with double
quotes (e.g., CHAIN_NAME="My Chain") so dotenv-compatible loaders parse it
reliably and consistently.
In `@backend/crates/atlas-api/src/handlers/status.rs`:
- Around line 25-43: The /api/status contract was changed incorrectly — restore
get_status to be the lightweight endpoint that performs a single key-value
lookup against the indexer_state table and returns { block_height, indexed_at }
(the same shape as HeightResponse) and remove or rename the separate full-info
endpoint; specifically, move the SQL query "SELECT value, updated_at FROM
indexer_state WHERE key = 'last_indexed_block'" and its parsing logic (currently
in get_height) into get_status, have get_status return Json<HeightResponse>
using result.0.parse() for block_height and result.1.to_rfc3339() for
indexed_at, and delete or non-routify the get_height function/route so there is
only one /api/status endpoint complying with the guideline.
In `@frontend/src/App.tsx`:
- Around line 38-43: Routes for the "tokens" and "tokens/:address" entries have
inconsistent indentation compared to the other <Route> siblings; align the JSX
so all <Route ... /> elements in App.tsx use the same indentation level (e.g.,
match the indentation used by <Route path="nfts" ... />, <Route path="status"
... /> and others). Edit the lines containing the <Route path="tokens"
element={<TokensPage />} /> and <Route path="tokens/:address"
element={<TokenDetailPage />} /> entries to remove the extra leading space and
ensure consistent indentation across the Route list.
---
Outside diff comments:
In `@backend/crates/atlas-api/src/handlers/status.rs`:
- Around line 27-63: Add a #[cfg(test)] mod tests block to this file containing
unit tests for get_height and get_status: mock or construct an Arc<AppState>
with a test sqlx::Sqlite/Postgres in-memory pool (or use sqlx::query mocking),
insert a row for key 'last_indexed_block', and assert that get_height returns
the expected HeightResponse and get_status returns ChainStatus with correct
block_height, indexed_at, total_transactions and total_addresses (exercise
get_table_count by creating test rows for "transactions" and "addresses" or
mocking it). Name tests clearly (e.g., test_get_height_returns_parsed_height,
test_get_status_returns_full_chain_status), import necessary types (AppState,
HeightResponse, ChainStatus, get_height, get_status, get_table_count) and run
them with cargo test --workspace to ensure compilation and behavior.
---
Nitpick comments:
In `@backend/crates/atlas-api/src/handlers/mod.rs`:
- Around line 55-58: The query interpolates table_name directly into sql and
should quote and escape the identifier for defense-in-depth: when building the
sql for sqlx::query_as (where variable table_name is used to produce exact),
wrap the identifier in double quotes and escape any embedded double quotes by
replacing " with "" before interpolation (or use sqlx's
QueryBuilder/identifier-quoting helper if available) so the final string becomes
a safely quoted table identifier instead of raw table_name.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2a6ab6a6-fbcc-4885-b4ae-d1effce5f286
📒 Files selected for processing (12)
.env.exampleCLAUDE.mdbackend/crates/atlas-api/src/handlers/mod.rsbackend/crates/atlas-api/src/handlers/status.rsbackend/crates/atlas-api/src/handlers/transactions.rsbackend/crates/atlas-api/src/main.rsdocker-compose.ymlfrontend/src/App.tsxfrontend/src/api/status.tsfrontend/src/components/Layout.tsxfrontend/src/pages/StatusPage.tsxfrontend/src/pages/index.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
backend/crates/atlas-api/src/main.rs (2)
292-340: Good test coverage addition; add one more case for missingresult.You covered success, invalid hex, and transport failure. Add a test for the Line 38-40 path where
resultis absent/null to fully cover error branches.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/crates/atlas-api/src/main.rs` around lines 292 - 340, Add a new async test in the tests module that uses serve_json_once to return a JSON-RPC response missing the "result" field (e.g. {"jsonrpc":"2.0","id":1}) and assert that fetch_chain_id(&url).await returns an error containing the expected message for a missing/null result; name it something like fetch_chain_id_returns_error_for_missing_result to match the existing test style and ensure it covers the code path handling absent result in fetch_chain_id.
80-84: Add bounded retry for startup chain ID fetch.Line 82 currently fails startup on the first transient RPC error. A short retry/backoff loop would improve boot reliability without changing fail-closed behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/crates/atlas-api/src/main.rs` around lines 80 - 84, Wrap the single call to fetch_chain_id(&rpc_url).await? in a bounded retry/backoff loop so transient RPC errors don't fail startup immediately: perform N attempts (e.g. 3-5) with an increasing delay (e.g. exponential backoff using tokio::time::sleep) between attempts, log each retry via tracing::info!/error! including attempt count and error, and return the original error only if all attempts are exhausted; update the startup block in main.rs where rpc_url and fetch_chain_id are used to implement this retry logic around fetch_chain_id.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/crates/atlas-api/src/main.rs`:
- Around line 263-264: Remove the duplicate status surface by deleting the
.route("/api/height", get(handlers::status::get_height)) registration and ensure
the single chain-status handler handlers::status::get_status is the only exposed
endpoint; update handlers::status::get_status (and any related types) to return
exactly the contract shape { block_height, indexed_at } so clients rely solely
on GET /api/status for chain status information.
---
Nitpick comments:
In `@backend/crates/atlas-api/src/main.rs`:
- Around line 292-340: Add a new async test in the tests module that uses
serve_json_once to return a JSON-RPC response missing the "result" field (e.g.
{"jsonrpc":"2.0","id":1}) and assert that fetch_chain_id(&url).await returns an
error containing the expected message for a missing/null result; name it
something like fetch_chain_id_returns_error_for_missing_result to match the
existing test style and ensure it covers the code path handling absent result in
fetch_chain_id.
- Around line 80-84: Wrap the single call to fetch_chain_id(&rpc_url).await? in
a bounded retry/backoff loop so transient RPC errors don't fail startup
immediately: perform N attempts (e.g. 3-5) with an increasing delay (e.g.
exponential backoff using tokio::time::sleep) between attempts, log each retry
via tracing::info!/error! including attempt count and error, and return the
original error only if all attempts are exhausted; update the startup block in
main.rs where rpc_url and fetch_chain_id are used to implement this retry logic
around fetch_chain_id.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7471d202-a2de-49c1-ab17-523418b305c0
📒 Files selected for processing (3)
.env.examplebackend/crates/atlas-api/src/main.rsfrontend/src/App.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- .env.example
- frontend/src/App.tsx
tac0turtle
reviewed
Mar 16, 2026
pthmas
force-pushed
the
pierrick/status-page
branch
from
15033ab to
783aaf0
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/crates/atlas-server/src/api/handlers/mod.rs (1)
20-57: 🛠️ Refactor suggestion | 🟠 MajorAdd in-file unit tests for the new counting logic.
This introduces new branching behavior and should include
#[cfg(test)] mod testscoverage in the same file (threshold path, parent fallback, and unsupported table handling if allowlisted).As per coding guidelines "
backend/**/*.rs: Add unit tests for new logic in a#[cfg(test)] mod testsblock in the same file, and run withcargo test --workspace."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/crates/atlas-server/src/api/handlers/mod.rs` around lines 20 - 57, Add an in-file test module (#[cfg(test)] mod tests) under the same file that contains async unit tests for get_table_count: one test that seeds/mocks pg_class+pg_inherits to produce a large approx_partitions sum (>100_000) and asserts get_table_count returns the approx value (threshold path), one test that leaves no partition rows so the parent fallback path (query selecting reltuples in parent) is used and the returned count matches the parent reltuples, and one test for the "unsupported table handling if allowlisted" scenario described by the review (mock or seed state to represent the allowlist and assert the expected behavior). Locate tests next to get_table_count and reference the same symbols (get_table_count, approx_partitions, parent) and ensure tests run with async runtime and are executed by cargo test --workspace.
🧹 Nitpick comments (1)
backend/crates/atlas-server/src/api/handlers/status.rs (1)
34-34: Consider logging parse failures instead of silently defaulting to 0.Using
.unwrap_or(0)masks potential data corruption or schema issues inindexer_state.value. Ablock_heightof 0 could be mistaken for "no blocks indexed yet" rather than "parse failed."♻️ Proposed improvement
- let block_height: i64 = result.0.parse().unwrap_or(0); + let block_height: i64 = result.0.parse().unwrap_or_else(|e| { + tracing::warn!("Failed to parse block height '{}': {}", result.0, e); + 0 + });Also applies to: 50-50
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/crates/atlas-server/src/api/handlers/status.rs` at line 34, The current parse of result.0 into block_height uses .unwrap_or(0), hiding parsing errors; replace it with explicit parse handling that logs the parse failure (including the offending string result.0 and the parse error) and only falls back to a sentinel or returns an error as appropriate; update the let block_height: i64 = result.0.parse().unwrap_or(0); line to use a match or Result-based flow that calls the request/handler logger (or error macro used in this module) to record the failure for indexer_state.value, and apply the same change to the other occurrence mentioned (around lines 50) so parsing problems aren’t silently turned into 0.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/crates/atlas-server/src/api/handlers/mod.rs`:
- Line 52: Replace the unsafe dynamic identifier interpolation used in the
sqlx::query_as call (format!("SELECT COUNT(*) FROM {table_name}")) by
whitelisting allowed table names with a match on the table_name variable and
mapping each branch to a hard-coded, prebuilt query string (or prepared
statement) so the code never interpolates identifiers at runtime; update the
code paths that set the exact: (i64,) result and any approximate-count logic to
use the chosen query string. Also add a #[cfg(test)] mod tests block in the same
file (mod.rs) containing unit tests that exercise both the approximate-count
branch (simulate reltuples > 100_000) and the exact-count fallback branch
(simulate reltuples < 100_000), asserting the expected behavior of the function
that uses sqlx::query_as and the exact variable; use test doubles or a test
database fixture as appropriate to isolate the two scenarios.
---
Outside diff comments:
In `@backend/crates/atlas-server/src/api/handlers/mod.rs`:
- Around line 20-57: Add an in-file test module (#[cfg(test)] mod tests) under
the same file that contains async unit tests for get_table_count: one test that
seeds/mocks pg_class+pg_inherits to produce a large approx_partitions sum
(>100_000) and asserts get_table_count returns the approx value (threshold
path), one test that leaves no partition rows so the parent fallback path (query
selecting reltuples in parent) is used and the returned count matches the parent
reltuples, and one test for the "unsupported table handling if allowlisted"
scenario described by the review (mock or seed state to represent the allowlist
and assert the expected behavior). Locate tests next to get_table_count and
reference the same symbols (get_table_count, approx_partitions, parent) and
ensure tests run with async runtime and are executed by cargo test --workspace.
---
Nitpick comments:
In `@backend/crates/atlas-server/src/api/handlers/status.rs`:
- Line 34: The current parse of result.0 into block_height uses .unwrap_or(0),
hiding parsing errors; replace it with explicit parse handling that logs the
parse failure (including the offending string result.0 and the parse error) and
only falls back to a sentinel or returns an error as appropriate; update the let
block_height: i64 = result.0.parse().unwrap_or(0); line to use a match or
Result-based flow that calls the request/handler logger (or error macro used in
this module) to record the failure for indexer_state.value, and apply the same
change to the other occurrence mentioned (around lines 50) so parsing problems
aren’t silently turned into 0.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cea1f2a7-5571-4daa-8506-630104f6b7e9
📒 Files selected for processing (14)
.env.exampleCLAUDE.mdbackend/crates/atlas-server/src/api/handlers/mod.rsbackend/crates/atlas-server/src/api/handlers/status.rsbackend/crates/atlas-server/src/api/handlers/transactions.rsbackend/crates/atlas-server/src/api/mod.rsbackend/crates/atlas-server/src/config.rsbackend/crates/atlas-server/src/main.rsdocker-compose.ymlfrontend/src/App.tsxfrontend/src/api/status.tsfrontend/src/components/Layout.tsxfrontend/src/pages/StatusPage.tsxfrontend/src/pages/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- .env.example
- frontend/src/pages/StatusPage.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/crates/atlas-server/src/api/handlers/mod.rs`:
- Around line 19-21: Remove the redundant `.into()` on the formatted string
passed to the `sqlx::Error::Protocol` constructor: the `format!("unsupported
table for exact count: {table_name}")` already yields a `String`, so update the
`Err(sqlx::Error::Protocol(...))` arm (the branch that returns the unsupported
table error) to pass the `String` directly to `sqlx::Error::Protocol` (leave the
`table_name` interpolation and surrounding message unchanged).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3416c1f9-2bb6-4801-a1c4-eace15a19cb7
📒 Files selected for processing (1)
backend/crates/atlas-server/src/api/handlers/mod.rs
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/crates/atlas-server/src/api/handlers/mod.rs (1)
34-69:⚠️ Potential issue | 🟠 MajorReject unsupported tables before the approximate-count branch.
Right now the whitelist only runs in the exact branch. An existing unsupported table above the threshold will return
Ok(approx)instead of erroring, soget_table_countchanges behavior based on row count. Validatetable_nameup front (or switch the parameter to an enum) so only the intended tables can use this helper.As per coding guidelines, `For large tables (transactions, addresses), use pg_class.reltuples for row count estimation instead of COUNT(*)`, with fallback to exact `COUNT(*)` for tables under 100k rows.🔧 Suggested fix
pub async fn get_table_count(pool: &PgPool, table_name: &str) -> Result<i64, sqlx::Error> { + let exact_sql = exact_count_sql(table_name)?; + // Sum approximate reltuples across partitions if any, else use parent. // This is instant and reasonably accurate for large tables. // Cast to float8 (f64) since reltuples is float4 and SUM returns float4 let approx_partitions: (Option<f64>,) = sqlx::query_as( r#" @@ if should_use_approximate_count(approx) { Ok(approx) } else { // Exact count for small tables - let exact: (i64,) = sqlx::query_as(exact_count_sql(table_name)?) + let exact: (i64,) = sqlx::query_as(exact_sql) .fetch_one(pool) .await?; Ok(exact.0) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/crates/atlas-server/src/api/handlers/mod.rs` around lines 34 - 69, get_table_count currently only validates the allowed/unsupported table names in the exact-count branch, so large unsupported tables can return Ok(approx); move or add the whitelist validation at the start of get_table_count (or change the function to accept a validated enum) so table_name is rejected immediately before computing approx_partitions or calling should_use_approximate_count; specifically, ensure the same whitelist check used in the exact branch is invoked at the top of get_table_count (before calling approx_partitions query) and return an error for unsupported names so only intended tables (e.g., transactions, addresses) can use reltuples-based estimation with fallback to exact_count_sql when should_use_approximate_count decides to use COUNT(*).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@backend/crates/atlas-server/src/api/handlers/mod.rs`:
- Around line 34-69: get_table_count currently only validates the
allowed/unsupported table names in the exact-count branch, so large unsupported
tables can return Ok(approx); move or add the whitelist validation at the start
of get_table_count (or change the function to accept a validated enum) so
table_name is rejected immediately before computing approx_partitions or calling
should_use_approximate_count; specifically, ensure the same whitelist check used
in the exact branch is invoked at the top of get_table_count (before calling
approx_partitions query) and return an error for unsupported names so only
intended tables (e.g., transactions, addresses) can use reltuples-based
estimation with fallback to exact_count_sql when should_use_approximate_count
decides to use COUNT(*).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cf4ec5ca-bf79-4308-a4e5-f6e761f106b3
📒 Files selected for processing (1)
backend/crates/atlas-server/src/api/handlers/mod.rs
…ight - Rename existing status endpoint to /api/height (block_height + indexed_at) - Add new /api/status with chain_id, chain_name, total_transactions, total_addresses - Fetch chain_id once from RPC at startup and store in AppState - Read CHAIN_NAME from env, defaults to Unknown - Make get_table_count accept a table_name parameter, reused for addresses
- Add StatusPage showing chain_id, chain_name, block_height, total_transactions, total_addresses - Add getChainStatus() API call to /api/status - Update getStatus() to call /api/height - Add route and nav link for status page
pthmas
force-pushed
the
pierrick/status-page
branch
from
58ce533 to
6a7d388
Compare
Collaborator
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/crates/atlas-server/src/api/handlers/transactions.rs (1)
14-29:⚠️ Potential issue | 🟠 MajorThe transaction list still paginates with
OFFSET.The new approximate total helps the metadata, but deep pages on this endpoint will still degrade into expensive scans because the hot query is unchanged. Since this PR already touched the transaction listing path, I'd switch it to keyset/cursor pagination before shipping.
As per coding guidelines: Never use
OFFSETfor pagination on large tables — use keyset/cursor pagination instead.
♻️ Duplicate comments (1)
backend/crates/atlas-server/src/api/handlers/status.rs (1)
44-59:⚠️ Potential issue | 🟠 Major
/api/statuscontract is still diverging from the project API rule.This still introduces
/api/heightas the lightweight poll endpoint while/api/statusbecame the heavy full-status endpoint. That concern was already raised earlier and appears unresolved in this revision.As per coding guidelines "
GET /api/statusendpoint for navbar polling fallback — returns{ block_height, indexed_at }via a single key-value lookup fromindexer_statetable (sub-ms latency)`."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/crates/atlas-server/src/api/handlers/status.rs` around lines 44 - 59, The get_status handler currently performs heavy counts (get_table_count on "transactions" and "addresses") and diverges from the project rule that GET /api/status must be the lightweight navbar-poll endpoint; change get_status (function get_status) to return only the minimal payload produced by latest_height_and_indexed_at (block_height and indexed_at) — e.g., reuse/latest_height_and_indexed_at(&state).await? and return a ChainStatus/HeightResponse-like object with just those fields — and move the expensive table counts into a separate endpoint (e.g., get_full_status or keep get_height) so AppState/ChainStatus semantics remain lightweight and sub-ms.
🧹 Nitpick comments (2)
frontend/src/api/status.ts (1)
17-24: RenamegetStatus()to match/height.Now that
/statusis the rich chain payload and/heightis the lightweight poller, exportinggetStatus()besidegetChainStatus()makes wrong imports easy. A name likegetHeight()would keep call sites self-describing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/api/status.ts` around lines 17 - 24, Rename the lightweight fetch function getStatus to getHeight to avoid confusion with getChainStatus: update the function declaration export async function getStatus() to export async function getHeight(), update any internal references and exports in the module, and update all call sites that import or call getStatus to use getHeight instead so imports remain self-describing and match the '/height' endpoint.backend/crates/atlas-server/src/api/handlers/status.rs (1)
106-131: Add tests for the new fallback branches inlatest_height_and_indexed_at.Current tests only validate the head-tracker path. Please also cover:
- head tracker empty +
indexer_staterow exists, and- neither source has data (default return path).
As per coding guidelines "Add unit tests for new logic in a
#[cfg(test)] mod testsblock in the same file, and run withcargo test --workspace."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/crates/atlas-server/src/api/handlers/status.rs` around lines 106 - 131, Add two unit tests in the same #[cfg(test)] mod tests block covering the fallback branches of latest_height_and_indexed_at: (1) when HeadTracker is empty but an indexer_state row exists, create an Arc::new(HeadTracker::empty(...)) and insert a persisted indexer_state row (using the same helpers your test suite exposes, e.g., test_state or whatever populates the DB) then call get_height(test_state(...)) and assert the returned status.block_height and indexed_at match the indexer_state values; (2) when neither HeadTracker nor indexer_state has data, use an empty HeadTracker and an empty DB (no indexer_state row), call get_height(test_state(...)) and assert the returned status.block_height is the default (likely 0) and indexed_at is empty; ensure tests reference latest_height_and_indexed_at behavior via get_height, reuse sample_block/HeadTracker helpers, and follow existing test patterns in this file so they run under cargo test --workspace.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/api/status.ts`:
- Around line 8-15: ChainStatusResponse currently types chain_id as number which
will lose precision for u64 values—change ChainStatusResponse.chain_id to string
(or bigint if you adopt bigint end-to-end) and update any callers to treat
chain_id as a string; also rename the lightweight polling function getStatus()
to getHeight() (update its signature and all imports/usages) and ensure
getHeight() returns only the lightweight fields block_height and indexed_at (not
the full ChainStatusResponse) so the name matches behavior; update related
types/usages that referenced getStatus or assumed numeric chain_id accordingly.
---
Duplicate comments:
In `@backend/crates/atlas-server/src/api/handlers/status.rs`:
- Around line 44-59: The get_status handler currently performs heavy counts
(get_table_count on "transactions" and "addresses") and diverges from the
project rule that GET /api/status must be the lightweight navbar-poll endpoint;
change get_status (function get_status) to return only the minimal payload
produced by latest_height_and_indexed_at (block_height and indexed_at) — e.g.,
reuse/latest_height_and_indexed_at(&state).await? and return a
ChainStatus/HeightResponse-like object with just those fields — and move the
expensive table counts into a separate endpoint (e.g., get_full_status or keep
get_height) so AppState/ChainStatus semantics remain lightweight and sub-ms.
---
Nitpick comments:
In `@backend/crates/atlas-server/src/api/handlers/status.rs`:
- Around line 106-131: Add two unit tests in the same #[cfg(test)] mod tests
block covering the fallback branches of latest_height_and_indexed_at: (1) when
HeadTracker is empty but an indexer_state row exists, create an
Arc::new(HeadTracker::empty(...)) and insert a persisted indexer_state row
(using the same helpers your test suite exposes, e.g., test_state or whatever
populates the DB) then call get_height(test_state(...)) and assert the returned
status.block_height and indexed_at match the indexer_state values; (2) when
neither HeadTracker nor indexer_state has data, use an empty HeadTracker and an
empty DB (no indexer_state row), call get_height(test_state(...)) and assert the
returned status.block_height is the default (likely 0) and indexed_at is empty;
ensure tests reference latest_height_and_indexed_at behavior via get_height,
reuse sample_block/HeadTracker helpers, and follow existing test patterns in
this file so they run under cargo test --workspace.
In `@frontend/src/api/status.ts`:
- Around line 17-24: Rename the lightweight fetch function getStatus to
getHeight to avoid confusion with getChainStatus: update the function
declaration export async function getStatus() to export async function
getHeight(), update any internal references and exports in the module, and
update all call sites that import or call getStatus to use getHeight instead so
imports remain self-describing and match the '/height' endpoint.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 164a3217-b474-41e9-aede-f9da3083dbe3
📒 Files selected for processing (15)
.env.exampleCLAUDE.mdbackend/crates/atlas-server/src/api/handlers/mod.rsbackend/crates/atlas-server/src/api/handlers/sse.rsbackend/crates/atlas-server/src/api/handlers/status.rsbackend/crates/atlas-server/src/api/handlers/transactions.rsbackend/crates/atlas-server/src/api/mod.rsbackend/crates/atlas-server/src/config.rsbackend/crates/atlas-server/src/main.rsdocker-compose.ymlfrontend/src/App.tsxfrontend/src/api/status.tsfrontend/src/components/Layout.tsxfrontend/src/pages/StatusPage.tsxfrontend/src/pages/index.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- .env.example
- frontend/src/components/Layout.tsx
- frontend/src/pages/StatusPage.tsx
- backend/crates/atlas-server/src/config.rs
- backend/crates/atlas-server/src/api/mod.rs
- docker-compose.yml
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
2 participants
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.
Overview
Adds a new /api/status endpoint returning chain ID, chain name, block height, total transactions, and total addresses, with chain name configurable via CHAIN_NAME env var. Renames the existing status endpoint to /api/height to better reflect its purpose, and adds a frontend status page displaying all chain information.
Summary by CodeRabbit
New Features
Chores