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 nullable block-level Changes
Sequence Diagram(s)sequenceDiagram
participant Indexer as Indexer
participant DB as Database
participant API as Atlas API
participant FE as Frontend
rect rgba(200,220,255,0.5)
Indexer->>DB: INSERT/UPDATE blocks (include base_fee_per_gas)
end
Note right of DB: blocks.base_fee_per_gas stored (nullable)
FE->>API: GET /stats/gas-price (chart)
API->>DB: Run bucketed SQL\n(tx.avg_gas_price, block.avg_base_fee_per_gas)
DB-->>API: Return bucket rows with nullable aggregates
API->>API: resolve_avg_gas_price(tx_avg, block_avg)\n(prefers tx, then block, else null)
API-->>FE: JSON with avg_gas_price (nullable)
FE->>FE: formatGwei(value:number|null)\nshow '—' for null/NaN
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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.
🧹 Nitpick comments (1)
backend/crates/atlas-server/src/api/handlers/stats.rs (1)
231-266: Consider adding a unit test for nullable serialization.The existing tests cover
Windowenum methods but don't validate the new nullableavg_gas_pricebehavior. A simple serialization test would help ensureOption<f64>serializes correctly to JSONnull.📝 Example test to add
+ #[test] + fn gas_price_point_serializes_null() { + let point = GasPricePoint { + bucket: "2024-01-01T00:00:00Z".to_string(), + avg_gas_price: None, + }; + let json = serde_json::to_string(&point).unwrap(); + assert!(json.contains(r#""avg_gas_price":null"#)); + }As per coding guidelines: "Add unit tests for new logic in a
#[cfg(test)] mod testsblock in the same file".🤖 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/stats.rs` around lines 231 - 266, Add a unit test in the existing #[cfg(test)] mod tests that verifies nullable avg_gas_price serializes to JSON null and Some(f64) serializes to a numeric value: create a small #[derive(Serialize, Deserialize, PartialEq, Debug)] temp struct with an avg_gas_price: Option<f64> field inside the tests module, use serde_json::to_string (and optionally from_str) to assert that None -> "null" for that field in the JSON and Some(1.23) -> numeric 1.23, and place the test alongside the existing Window tests so it runs with cargo test; reference avg_gas_price, serde_json::to_string, and the tests module when locating where to add it.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@backend/crates/atlas-server/src/api/handlers/stats.rs`:
- Around line 231-266: Add a unit test in the existing #[cfg(test)] mod tests
that verifies nullable avg_gas_price serializes to JSON null and Some(f64)
serializes to a numeric value: create a small #[derive(Serialize, Deserialize,
PartialEq, Debug)] temp struct with an avg_gas_price: Option<f64> field inside
the tests module, use serde_json::to_string (and optionally from_str) to assert
that None -> "null" for that field in the JSON and Some(1.23) -> numeric 1.23,
and place the test alongside the existing Window tests so it runs with cargo
test; reference avg_gas_price, serde_json::to_string, and the tests module when
locating where to add it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b221d7f7-91af-4127-820a-abd7cf2f17e0
📒 Files selected for processing (4)
backend/crates/atlas-server/src/api/handlers/stats.rsfrontend/src/api/chartData.tsfrontend/src/hooks/useChartData.tsfrontend/src/pages/StatusPage.tsx
pthmas
changed the title
Contributor
|
can you share a screen shot of before and after? |
pthmas
changed the title
There was a problem hiding this comment.
🧹 Nitpick comments (1)
backend/crates/atlas-server/src/api/handlers/stats.rs (1)
78-81: Breaking change to API response type — verify frontend alignment.Changing
avg_gas_pricefromf64toOption<f64>means the JSON field can now benullinstead of always a number. The AI summary indicates frontend changes handle this, but ensure consumers are updated.🤖 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/stats.rs` around lines 78 - 81, You changed the API shape by making GasPricePoint.avg_gas_price an Option<f64>, which allows nulls; either revert it to f64 to preserve the original contract or if you intend nulls, update all consumers and API metadata: update the OpenAPI/Swagger spec and docs, modify frontend code to handle null avg_gas_price (and related parsing/formatting), and add appropriate serde behavior on the GasPricePoint type (e.g., default/skip_serializing_if) so serialization is explicit—ensure GasPricePoint and avg_gas_price are updated consistently across server, docs, and frontend.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@backend/crates/atlas-server/src/api/handlers/stats.rs`:
- Around line 78-81: You changed the API shape by making
GasPricePoint.avg_gas_price an Option<f64>, which allows nulls; either revert it
to f64 to preserve the original contract or if you intend nulls, update all
consumers and API metadata: update the OpenAPI/Swagger spec and docs, modify
frontend code to handle null avg_gas_price (and related parsing/formatting), and
add appropriate serde behavior on the GasPricePoint type (e.g.,
default/skip_serializing_if) so serialization is explicit—ensure GasPricePoint
and avg_gas_price are updated consistently across server, docs, and frontend.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a0a2ddaf-58b9-406b-8229-b8b85f99e857
📒 Files selected for processing (12)
backend/crates/atlas-common/src/types.rsbackend/crates/atlas-server/src/api/handlers/blocks.rsbackend/crates/atlas-server/src/api/handlers/search.rsbackend/crates/atlas-server/src/api/handlers/sse.rsbackend/crates/atlas-server/src/api/handlers/stats.rsbackend/crates/atlas-server/src/api/handlers/status.rsbackend/crates/atlas-server/src/head.rsbackend/crates/atlas-server/src/indexer/batch.rsbackend/crates/atlas-server/src/indexer/copy.rsbackend/crates/atlas-server/src/indexer/indexer.rsbackend/migrations/20260407000002_add_blocks_base_fee_per_gas.sqlfrontend/src/types/index.ts
✅ Files skipped from review due to trivial changes (5)
- backend/crates/atlas-server/src/api/handlers/sse.rs
- backend/crates/atlas-server/src/api/handlers/status.rs
- frontend/src/types/index.ts
- backend/migrations/20260407000002_add_blocks_base_fee_per_gas.sql
- backend/crates/atlas-server/src/head.rs
…hart # Conflicts: # frontend/src/pages/StatusPage.tsx
There was a problem hiding this comment.
🧹 Nitpick comments (1)
backend/crates/atlas-server/src/api/handlers/stats.rs (1)
290-303: Consider adding test forSome/Nonecase.The tests cover preference, fallback, and empty-bucket scenarios well. For completeness, consider adding a test for the case where transaction average exists but block base fee is
None:🧪 Proposed additional test
#[test] fn resolve_avg_gas_price_returns_none_when_bucket_is_empty() { assert_eq!(resolve_avg_gas_price(None, None), None); } + + #[test] + fn resolve_avg_gas_price_uses_tx_avg_when_no_block_base_fee() { + assert_eq!(resolve_avg_gas_price(Some(42.0), None), Some(42.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/stats.rs` around lines 290 - 303, Add a unit test for resolve_avg_gas_price that verifies when the transaction average is Some(value) and the block base fee is None the function still returns the transaction average; locate the test module containing resolve_avg_gas_price (existing tests like resolve_avg_gas_price_prefers_transaction_average) and add a new test function (e.g., resolve_avg_gas_price_with_tx_some_block_none) that asserts resolve_avg_gas_price(Some(42.0), None) == Some(42.0).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@backend/crates/atlas-server/src/api/handlers/stats.rs`:
- Around line 290-303: Add a unit test for resolve_avg_gas_price that verifies
when the transaction average is Some(value) and the block base fee is None the
function still returns the transaction average; locate the test module
containing resolve_avg_gas_price (existing tests like
resolve_avg_gas_price_prefers_transaction_average) and add a new test function
(e.g., resolve_avg_gas_price_with_tx_some_block_none) that asserts
resolve_avg_gas_price(Some(42.0), None) == Some(42.0).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4007c168-7658-4956-bc6f-1b2b9157c778
📒 Files selected for processing (2)
backend/crates/atlas-server/src/api/handlers/stats.rsdocker-compose.yml
💤 Files with no reviewable changes (1)
- docker-compose.yml
There was a problem hiding this comment.
🧹 Nitpick comments (1)
backend/crates/atlas-server/src/api/handlers/stats.rs (1)
288-302: Good test coverage for the new resolution logic.The three test cases appropriately cover:
- Transaction average takes precedence when both are present
- Falls back to base fee when transaction average is missing
- Returns
Nonewhen both sources are emptyConsider adding an edge case test for when transaction average is
Some(0.0)to verify it's still preferred over a non-zero base fee (sinceSome(0.0)is truthy in the.or()chain).🧪 Optional: edge case test for zero transaction average
#[test] fn resolve_avg_gas_price_returns_none_when_bucket_is_empty() { assert_eq!(resolve_avg_gas_price(None, None), None); } + + #[test] + fn resolve_avg_gas_price_prefers_zero_tx_avg_over_base_fee() { + // Some(0.0) is still a valid transaction average and should be preferred + assert_eq!(resolve_avg_gas_price(Some(0.0), Some(100.0)), Some(0.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/stats.rs` around lines 288 - 302, Add an edge-case unit test for resolve_avg_gas_price that verifies a transaction average of Some(0.0) is preferred over a non-zero base fee (e.g., resolve_avg_gas_price(Some(0.0), Some(7.0)) returns Some(0.0)); locate the tests near the existing resolve_avg_gas_price tests and add a descriptive test function name like resolve_avg_gas_price_prefers_zero_transaction_average to ensure the function's Some(0.0) value isn't incorrectly treated as absent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@backend/crates/atlas-server/src/api/handlers/stats.rs`:
- Around line 288-302: Add an edge-case unit test for resolve_avg_gas_price that
verifies a transaction average of Some(0.0) is preferred over a non-zero base
fee (e.g., resolve_avg_gas_price(Some(0.0), Some(7.0)) returns Some(0.0));
locate the tests near the existing resolve_avg_gas_price tests and add a
descriptive test function name like
resolve_avg_gas_price_prefers_zero_transaction_average to ensure the function's
Some(0.0) value isn't incorrectly treated as absent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 743a1512-4124-4e76-85ae-ffaa14d732e5
📒 Files selected for processing (1)
backend/crates/atlas-server/src/api/handlers/stats.rs
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.
Index block base_fee_per_gas so the gas price chart has a protocol-level fallback.
Use average transaction gas price when transactions exist, and base fee when a bucket is empty.
Summary by CodeRabbit
New Features
Improvements
Chores
Tests