feat: view list metadata by height#1141
Conversation
WalkthroughAdds a new SQL action to list metadata by block height with pagination and optional ref filtering, integrates test utilities and tests for that action, updates table assertion logic to support excluded columns, and appends a trailing newline to an existing migration file. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant T as Tests (Go)
participant U as Test Utils
participant E as Engine
participant A as Action:list_metadata_by_height
participant DB as DB (metadata)
T->>U: ListMetadataByHeight(input)
U->>U: Build TxContext (height, signer, caller)
U->>E: Call action list_metadata_by_height(params)
E->>A: Execute action with params
A->>A: Compute effective_from/to, validate range, clamp pagination
A->>DB: SELECT metadata WHERE key, optional ref, disabled_at IS NULL, created_at BETWEEN bounds ORDER BY created_at
DB-->>A: Stream rows
A-->>E: Stream rows to engine
E-->>U: Row callbacks
U-->>T: Result rows or error
note right of A: On invalid range -> error returned
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Time Submission Status
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
internal/migrations/021-metadata-actions.sql (4)
35-48: Enforce an upper bound on limit for safety/perf parity with list_streams.
list_streamscaps/errs at 5000. Consider mirroring that here to prevent accidental full scans.Apply:
if $limit IS NULL { $limit := 1000; } + if $limit > 5000 { + ERROR('Limit exceeds maximum allowed value of 5000'); + } if $offset IS NULL { $offset := 0; }
62-80: Add deterministic tiebreaker to ORDER BY.Multiple rows can share the same
created_atheight. Addrow_idas a stable secondary sort to avoid pagination flakiness.- ORDER BY created_at ASC + ORDER BY created_at ASC, row_id ASC LIMIT $limit OFFSET $offset;
17-24: Clarify/validate required parameters and visibility semantics.
- Should
$keybe mandatory? TodayWHERE metadata_key = $keyreturns no rows if$keyis NULL. If this is unintended, return a clear error when$keyis NULL/empty.- Should this listing respect stream
read_visibility(public/private) like Explorer endpoints do? If not, confirm that exposing metadata across all streams is acceptable.
71-79: Indexing check for query path.To keep this fast at scale, ensure an index like
(metadata_key, created_at)(and optionallyvalue_ref) exists, given the filter + order used here.tests/streams/utils/procedure/types.go (1)
162-172: Name alignment and parameter clarity.
- Nit: fix spacing on
Keyto match surrounding style.- Consider renaming
Value→Refto reflect$refin the SQL and avoid confusion with typed value columns (value_i, etc.). This is optional but improves readability.type ListMetadataByHeightInput struct { Platform *kwilTesting.Platform - Key string + Key string - Value *string + // Ref corresponds to SQL parameter $ref (matches metadata.value_ref) + // Ref *string // optional future rename; would require callsite updates + Value *string FromHeight *int64 ToHeight *int64 Limit *int Offset *int Height int64 }tests/streams/query/metadata_test.go (1)
265-326: Optional: factor out duplicated seeding across tests.The three by-height tests repeat the same insert loop. Extract a small helper to seed
metadataItemsto reduce noise.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
internal/migrations/001-common-actions.sql(1 hunks)internal/migrations/021-metadata-actions.sql(1 hunks)tests/streams/query/metadata_test.go(4 hunks)tests/streams/utils/procedure/execute.go(1 hunks)tests/streams/utils/procedure/types.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
tests/streams/utils/procedure/execute.go (1)
tests/streams/utils/procedure/types.go (2)
ListMetadataByHeightInput(163-172)ResultRow(32-32)
tests/streams/query/metadata_test.go (6)
tests/streams/utils/utils.go (1)
RunSchemaTest(372-407)tests/streams/utils/setup/common.go (1)
StreamInfo(20-23)tests/streams/utils/procedure/metadata.go (2)
InsertMetadata(293-328)InsertMetadataInput(283-290)tests/streams/utils/procedure/execute.go (1)
ListMetadataByHeight(848-888)tests/streams/utils/procedure/types.go (1)
ListMetadataByHeightInput(163-172)tests/streams/utils/table/assert.go (2)
AssertResultRowsEqualMarkdownTable(18-81)AssertResultRowsEqualMarkdownTableInput(11-16)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: acceptance-test
- GitHub Check: lint
🔇 Additional comments (3)
internal/migrations/001-common-actions.sql (1)
1322-1322: No functional change — OK to merge.Only a trailing newline added; nothing else modified.
tests/streams/utils/procedure/execute.go (1)
847-888: LGTM — engine call wiring and context setup look consistent with existing helpers.tests/streams/query/metadata_test.go (1)
36-47: Nice test coverage expansion.Good additions covering happy path, missing key, pagination, invalid ranges, and negative pagination.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
tests/streams/query/metadata_test.go (2)
207-212: Fix Markdown table delimiter row (malformed).The separator has the wrong number of columns. Make it match the 7 headers.
- |---------------|-----------|---------------------|-----------------|--------|------------|----------------|------------|------------|------------| + |------------|---------|---------|---------|---------|-----------|------------|
257-260: Bug: wrapping a nil error returns nil — test won’t fail.Use a new error to fail the test when results are unexpectedly non-empty.
- if (len(result) > 0) { // should return no rows - return errors.Wrapf(err, "expected empty results") - } + if len(result) > 0 { // should return no rows + return errors.New("expected empty results") + }
🧹 Nitpick comments (5)
tests/streams/utils/table/assert.go (2)
25-39: Make row filtering robust (avoid hard-coded column index).Skipping rows via
row[1] != ""is brittle for 1-column tables. Check for any non-empty cell instead.- for _, row := range expectedTable.Rows { - if row[1] != "" { - transformedRow := make([]string, len(row)) - for colIdx, value := range row { - colName := expectedTable.Headers[colIdx] - if transformer, exists := input.ColumnTransformers[colName]; exists && transformer != nil { - transformedRow[colIdx] = transformer(value) - } else { - transformedRow[colIdx] = value - } - } - expected = append(expected, transformedRow) - } - } + for _, row := range expectedTable.Rows { + hasValue := false + for _, v := range row { + if v != "" { hasValue = true; break } + } + if !hasValue { continue } + transformedRow := make([]string, len(row)) + for colIdx, value := range row { + colName := expectedTable.Headers[colIdx] + if transformer, exists := input.ColumnTransformers[colName]; exists && transformer != nil { + transformedRow[colIdx] = transformer(value) + } else { + transformedRow[colIdx] = value + } + } + expected = append(expected, transformedRow) + }
16-17: Clarify index space of ExcludedColumns.Document that indices are relative to ACTUAL rows (pre-exclusion), not expected headers.
- ExcludedColumns []int // drop only from actual + ExcludedColumns []int // indices relative to ACTUAL rows; drop only from actualtests/streams/query/metadata_test.go (3)
213-217: Stabilize order: sort by value and created_at in the assertion.Avoid relying on DB default ordering.
table.AssertResultRowsEqualMarkdownTable(t, table.AssertResultRowsEqualMarkdownTableInput{ Actual: result, Expected: expected, ExcludedColumns: []int{1}, + SortColumns: []string{"value_i", "created_at"}, })
292-307: Strengthen pagination test by asserting content/order, not just counts.Counts alone won’t catch ordering bugs. Consider asserting expected rows or using the table assertion with
SortColumns.
406-431: Clarify expectations for negative pagination values.
- For negative limit you check for empty results — good.
- For negative offset you only assert no error; also assert that it’s coerced to 0 (e.g., result length equals total matching rows).
Would you like a follow-up patch to assert the expected length based on the inserted rows?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
tests/streams/query/metadata_test.go(4 hunks)tests/streams/utils/table/assert.go(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/streams/query/metadata_test.go (6)
tests/streams/utils/utils.go (1)
RunSchemaTest(372-407)tests/streams/utils/setup/common.go (1)
StreamInfo(20-23)tests/streams/utils/procedure/metadata.go (2)
InsertMetadata(293-328)InsertMetadataInput(283-290)tests/streams/utils/procedure/execute.go (1)
ListMetadataByHeight(848-888)tests/streams/utils/procedure/types.go (1)
ListMetadataByHeightInput(163-172)tests/streams/utils/table/assert.go (2)
AssertResultRowsEqualMarkdownTable(19-99)AssertResultRowsEqualMarkdownTableInput(11-17)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: acceptance-test
- GitHub Check: lint
🔇 Additional comments (1)
tests/streams/query/metadata_test.go (1)
356-374: LGTM: invalid range negative test is precise.Error presence and message assertion look good.
2 participants
Related Problem
resolves: https://github.com/trufnetwork/trufscan/issues/32
How Has This Been Tested?
migrated
Summary by CodeRabbit
New Features
Tests
Style