feat(subgraph/web): time travel query refactor #1939
Conversation
WalkthroughThis pull request introduces a new Changes
Sequence Diagram(s)sequenceDiagram
participant D as Dispute Creation Handler
participant C as CourtCounter Entity
participant U as updateCourtCumulativeMetric
participant A as Appeal Decision Handler
participant J as Juror Stake Updater
participant S as updateCourtStateVariable
D->>C: Increment numberDisputes
D->>U: Update "numberDisputes" metric with delta & timestamp
A->>C: Update numberVotes
A->>U: Update "numberVotes" metric with delta & timestamp
J->>S: Update effectiveStake state with new value & timestamp
sequenceDiagram
participant UI as Web UI
participant Hook as useHomePageBlockQuery
participant Server as GraphQL Server
UI->>Hook: Request past court data with pastTimestamp
Hook->>Server: Execute query using pastTimestamp (via courtCounters)
Server-->>Hook: Return updated data
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (15)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
❌ Deploy Preview for kleros-v2-testnet failed. Why did it fail? →
|
❌ Deploy Preview for kleros-v2-neo failed. Why did it fail? →
|
❌ Deploy Preview for kleros-v2-university failed. Why did it fail? →
|
✅ Deploy Preview for kleros-v2-testnet-devtools ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
subgraph/core/src/datapoint.ts (1)
95-132: Consider validation for unrecognized metrics.
This new function correctly updates the “numberDisputes” and “numberVotes” fields of theCourtCounterentity, along with creating a daily snapshot. However, if an unexpectedmetricvalue is passed, the function silently does nothing. Consider handling unknown metrics explicitly (e.g., logging, throwing, or ignoring with justification) to avoid silent errors.web/src/hooks/queries/useHomePageBlockQuery.ts (3)
55-60: Guard against empty arrays when calling[0].
Although these utility functions look clean, calling[0]on an empty array will yieldundefined. Consider returning early or defaulting to a sentinel object ifcourtscan be empty.
77-77: Rename Set to maintain clarity.
Using aSet<number>to track processed indices is fine. However, naming itprocessedIndicesor similar could improve clarity in this block.
94-100: Reduce repeated function calls to improve performance.
Repeatedly callingprocessCourt(parentIndex)can be costly since it’s invoked multiple times in the same object literal. Store the return value in a local variable first to avoid repeated recursion.const parentCourt = processCourt(parentIndex); processedCourts[id] = { ...courtWithTree, treeNumberDisputes: courtWithTree.treeNumberDisputes + parentCourt.treeNumberDisputes, treeNumberVotes: courtWithTree.treeNumberVotes + parentCourt.treeNumberVotes, // ... };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
subgraph/core/schema.graphql(1 hunks)subgraph/core/src/KlerosCore.ts(3 hunks)subgraph/core/src/datapoint.ts(2 hunks)subgraph/core/src/entities/JurorTokensPerCourt.ts(2 hunks)web/src/hooks/queries/useHomePageBlockQuery.ts(5 hunks)web/src/hooks/queries/useHomePageExtraStats.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
web/src/hooks/queries/useHomePageBlockQuery.ts (1)
Learnt from: Harman-singh-waraich
PR: kleros/kleros-v2#1744
File: web/src/hooks/queries/useHomePageBlockQuery.ts:71-71
Timestamp: 2025-04-03T18:37:26.185Z
Learning: In `web/src/hooks/queries/useHomePageBlockQuery.ts`, the non-null assertions on `blockNumber!` and `genesisBlock!` within `queryFn` are safe because `isEnabled` ensures that `queryFn` only runs when either `blockNumber` or `genesisBlock` is defined.
🧬 Code Definitions (3)
subgraph/core/src/entities/JurorTokensPerCourt.ts (1)
subgraph/core/src/datapoint.ts (1)
updateCourtStateVariable(134-168)
subgraph/core/src/KlerosCore.ts (1)
subgraph/core/src/datapoint.ts (1)
updateCourtCumulativeMetric(96-132)
web/src/hooks/queries/useHomePageExtraStats.ts (1)
web/src/hooks/queries/useHomePageBlockQuery.ts (1)
useHomePageBlockQuery(170-187)
⏰ Context from checks skipped due to timeout of 90000ms (15)
- GitHub Check: Redirect rules - kleros-v2-neo
- GitHub Check: Redirect rules - kleros-v2-university
- GitHub Check: Header rules - kleros-v2-neo
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-university
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-neo
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-university
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: Analyze (javascript)
- GitHub Check: SonarCloud
- GitHub Check: contracts-testing
🔇 Additional comments (18)
subgraph/core/schema.graphql (1)
245-252: LGTM! A well-designed entity for court metrics tracking.The addition of the
CourtCounterentity is well-structured with appropriate fields for storing cumulative metrics and timestamps. It will provide valuable time-series data points for court metrics.subgraph/core/src/entities/JurorTokensPerCourt.ts (2)
3-3: Appropriate import for the newupdateCourtStateVariablefunction.The import is correctly added to bring in the required function for updating court state variables.
97-97: Good integration of court state tracking.The addition of
updateCourtStateVariablecall ensures that whenever a juror's stake is updated, the court's effective stake is properly tracked in the new time-series data structure. This will allow for historical queries of court metrics.subgraph/core/src/KlerosCore.ts (4)
26-26: Appropriate import for the new update function.The import is correctly added to bring in the required function for updating court cumulative metrics.
85-85: Good addition for dispute creation tracking.The call to
updateCourtCumulativeMetricensures that new disputes are properly recorded in the time-series data structure, allowing for historical querying of dispute counts.
89-89: Proper tracking of votes during dispute creation.This tracking of votes at dispute creation time enhances the system's ability to provide historical court metrics related to voting activities.
231-231: Consistent tracking of votes during appeal decisions.The addition of vote tracking during appeal decisions ensures that all vote metrics are consistently captured in the time-series data, regardless of when they occur in the dispute lifecycle.
web/src/hooks/queries/useHomePageExtraStats.ts (3)
8-8: Good shift from block-based to timestamp-based tracking.Changing from
pastBlockNumbertopastTimestampsimplifies the data model and improves the accuracy of time-based queries.
11-16: Improved time calculation logic.The new implementation directly calculates the past timestamp based on the current time, which is more intuitive and eliminates dependencies on block-related calculations.
19-19: Updated query to use timestamp-based filtering.The modified call to
useHomePageBlockQueryproperly passes the calculated timestamp instead of a block number, completing the transition to timestamp-based historical queries.subgraph/core/src/datapoint.ts (1)
2-2: Ensure consistent usage of imported entities.
The addition ofCourtCounteralongsideCounteris consistent with the changes below, where these entities are referenced. No concerns here.web/src/hooks/queries/useHomePageBlockQuery.ts (7)
8-8: Timestamp-based query parameter looks good.
Changing the query to accept$pastTimestampaligns with the new time travel approach. Verify that all references to the oldblockNumberparameter in downstream code have been removed or updated.
20-20: Querying courtCounters by timestamp is consistent with the new logic.
ThepastCourtsfield now relies ontimestamp_lte: $pastTimestamp. This is appropriate for the new time-travel functionality. Ensure that descending order bytimestampis indeed the intended behavior, especially if multiple snapshots exist on the same day.
32-32: Type alias for CourtCounter is a helpful addition.
Creating a dedicatedCourtCountertype reference from GraphQL results is good for clarity, ensuring type safety across the codebase.
66-74: Map builder for past courts works as intended.
Storing only the first encountered court snapshot (descending timestamp) ensures we capture the most recent state when!allTime. This logic seems correct, but be mindful of concurrency if multiple snapshots share the same exact timestamp to avoid overwriting.
83-90: Conditional usage of diff logic is appropriate.
UsingaddTreeValuesWithDiffonly when a matchingpastCourtsnapshot is found ensures that you account for partial historical data. This aligns well with the newly introducedCourtCounterlogic.
141-168: Be mindful of integer truncation with BigInt averaging.
Using(presentCourtWithTree.effectiveStake + pastEffectiveStake) / 2ntruncates decimals if the sum is odd. This is likely acceptable, but confirm this is intentional. If you want a more precise average, you might need floating arithmetic.
170-187: New time-based hook logic looks solid.
The updateduseHomePageBlockQueryhook thoroughly replaces block-number logic with timestamp logic. It is conditionally enabled only if a validpastTimestampis provided orallTimeis true. Confirm that providingpastTimestamp= 0 does not cause unexpected queries for negative or null time ranges.
|
Code Climate has analyzed commit d5788be and detected 13 issues on this pull request. Here's the issue category breakdown:
View more on Code Climate. |
|
needs subgraph redeployment (or deploy preview will fail)
luckily I had time to compare the data before they deprecate the time travel query (which they will in a couple of hours). everything matches. tested on my personal subgraph.
Screen.Recording.2025-04-03.at.20.32.03.mov
PR-Codex overview
This PR updates the
@kleros/kleros-v2-subgraphpackage version and enhances the schema and functionality related toCourtCounter, adding metrics tracking for disputes and votes. It also refines the handling of timestamps and queries in the frontend.Detailed summary
versioninsubgraph/package.jsonto0.14.0.CourtCounterentity insubgraph/core/schema.graphqlfor tracking court metrics.updateCourtCumulativeMetricandupdateCourtStateVariablefunctions insubgraph/core/src/datapoint.ts.updateJurorStaketo include updates for cumulative metrics.handleDisputeCreationandhandleAppealDecisionto update cumulative metrics.useHomePageExtraStatshook to use timestamps instead of block numbers.useHomePageBlockQueryto query based onpastTimestampinstead of block number.addTreeValuesWithDiffto handleCourtCounterfor past metrics.Summary by CodeRabbit
New Features
Chores