Feat/Waveform Architecture Decision Record - #550
Merged
OlufunbiIK merged 15 commits intoApr 27, 2026
Conversation
…jotel-dev/tip-tune into feature/canonical-env-reference
…rchModule, and tests
…are valid without casts
Create WAVEFORM_ARCHITECTURE.md ADR that records the decision to consolidate the duplicate mount-waveform module into the canonical src/waveform directory. The ADR documents history, ownership, current architecture, and future cleanup tasks. Also update: - backend/src/waveform/README.md: add consolidation note and architecture reference - backend/README.md: add Waveform to features, API endpoints, and project structure
|
@jotel-dev is attempting to deploy a commit to the olufunbiik's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThis PR establishes a canonical environment variable reference ( Changes
Sequence DiagramssequenceDiagram
participant Page as TipHistoryPage
participant Source as TipHistorySource<br/>(Interface)
participant Fixture as FixtureTipHistorySource
participant Api as ApiTipHistorySource
participant Service as tipService<br/>(Backend API)
Page->>Source: getSentTips(filters?, page?, pageSize?)
alt Data Source Type
Source->>Fixture: fixture implementation
Fixture-->>Page: hardcoded TipHistoryItem[]
else
Source->>Api: API implementation
Api->>Service: tipService.getReceivedHistory()
Service-->>Api: raw API response
Api->>Api: mapAndFilterResponse()
Api->>Api: applyFiltersAndSort()
Api-->>Page: TipHistoryItem[] + pagination
end
Page->>Page: render filtered results
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
Replace deprecated LedgerInfo struct initialization with individual setter methods (set_sequence_number, set_timestamp, set_network_id, set_base_reserve, set_min_persistent_entry_ttl, set_min_temp_entry_ttl, set_max_entry_ttl) to match soroban-sdk 21.7.0 API. Remove unused LedgerInfo import.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
contracts/auto-royalty-distribution/src/queries.rs (1)
8-23:⚠️ Potential issue | 🟠 MajorMajor: function silently returns empty results, masking unimplemented behavior.
Renaming
payout_idto_payout_idsilences the unused-parameter warning but turnsget_settlements_by_payout_idinto a function that always returnsOk(Vec::new(&env))regardless of input. The public contract methodAutoRoyaltyDistribution::get_settlements_by_payoutincontracts/auto-royalty-distribution/src/lib.rs:210-216still forwards a realpayout_idand its callers will interpret the empty result as "no settlements for this payout" — a false negative with no error signal.If the lookup cannot be implemented in this PR, fail loudly rather than silently returning empty data: return a dedicated error (e.g.,
Error::NotImplemented) or remove the public entrypoint until it is implemented. Also, the inline comment block reads as a TODO / scratch note and should be replaced with a real doc comment explaining the contract.🛡️ Suggested change: surface the missing implementation
/// Fetch all settlement records for a given payout ID -/// NOTE: This is an expensive operation and only searches within the current retention window. +/// NOTE: Not yet implemented — Soroban does not expose key iteration, and there is no +/// global index of tracks. Returns `Error::NotImplemented` until an index is added. pub fn get_settlements_by_payout_id( env: Env, - _payout_id: String, + _payout_id: String, ) -> Result<Vec<DistributionRecord>, Error> { - let settlements = Vec::new(&env); - - // This is still inherently difficult without a global index or iterating over all tracks. - // Since we don't have a list of all tracks, this function's original implementation was likely - // assuming it could iterate over all storage keys. In Soroban, iterating over all keys - // is only possible in certain environments or with specific storage setups. - - // For now, we'll keep it as a placeholder or implement it if there's a way to track all tracks. - // Given the current architecture, we can't easily find all tracks. - - Ok(settlements) + let _ = env; + Err(Error::NotImplemented) }(
Error::NotImplementedwill need to be added to theErrorenum, or substitute an existing appropriate variant.)Want me to wire up an
Error::NotImplementedvariant and update the test that exercises this path, or alternatively sketch a track-index-based implementation that would let this query work correctly?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contracts/auto-royalty-distribution/src/queries.rs` around lines 8 - 23, get_settlements_by_payout_id currently ignores its payout_id and always returns an empty Vec, masking unimplemented behavior; change it to return a clear error instead (e.g., add Error::NotImplemented to the Error enum and return Err(Error::NotImplemented) from get_settlements_by_payout_id), update AutoRoyaltyDistribution::get_settlements_by_payout to propagate that error rather than treating an empty Vec as “no settlements”, and replace the scratch comments with a proper doc comment on get_settlements_by_payout_id explaining the missing implementation and intended behavior.backend/src/waveform/README.md (1)
93-131:⚠️ Potential issue | 🟠 MajorAPI endpoint paths are out of sync with current controller implementation.
This README documents
/api/waveform/:trackIdpaths, but the controller uses/api/v1/tracks/:trackId/waveformand/api/v1/tracks/:trackId/waveform/regenerate. This can mislead integrators and break client setup.🔧 Proposed endpoint doc fix
-GET /api/waveform/:trackId +GET /api/v1/tracks/:trackId/waveform ... -GET /api/waveform/:trackId/status +GET /api/v1/tracks/:trackId/waveform ... -POST /api/waveform/:trackId/regenerate +POST /api/v1/tracks/:trackId/waveform/regenerate🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/waveform/README.md` around lines 93 - 131, Update the README endpoint paths to match the controller routes: replace GET /api/waveform/:trackId with GET /api/v1/tracks/:trackId/waveform, GET /api/waveform/:trackId/status with GET /api/v1/tracks/:trackId/waveform/status, and POST /api/waveform/:trackId/regenerate with POST /api/v1/tracks/:trackId/waveform/regenerate; keep the existing response schemas (id, trackId, waveformData, dataPoints, peakAmplitude, generationStatus, processingDurationMs, createdAt, updatedAt and status/retryCount) unchanged and ensure examples reflect the new paths so docs align with the controller routes handling waveform generation.
🧹 Nitpick comments (7)
docs/environment-reference.md (1)
31-34: Mark fallback secrets as development-only.
JWT_SECRET/EMBED_SECRETdefault examples look production-unsafe unless explicitly scoped. Add a “dev-only fallback” note to reduce accidental insecure deployments.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/environment-reference.md` around lines 31 - 34, Update the environment reference table entries for JWT_SECRET and EMBED_SECRET to explicitly mark their example values as development-only fallbacks: change the description or add a parenthetical to the variable names/descriptions (e.g., "JWT_SECRET (dev-only fallback)" and "EMBED_SECRET (dev-only fallback)") and update the example values to indicate they are unsafe for production (e.g., `dev-jwt-secret-placeholder` / `dev-embed-secret-placeholder`), plus add a short inline note below the table clarifying that these examples must be overridden with secure secrets in production. Ensure the edits touch the `JWT_SECRET` and `EMBED_SECRET` rows and the table caption/footnote in environment-reference.md.backend/RATE_LIMITING_QUICK_REFERENCE.md (1)
13-13: Tiny wording polish: hyphenate “rate-limiting configuration”.Optional readability nit only.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/RATE_LIMITING_QUICK_REFERENCE.md` at line 13, The sentence "TipTune uses environment variables for Redis and rate limiting configuration." needs a tiny wording polish: hyphenate "rate limiting configuration" to "rate-limiting configuration"; locate the exact sentence in RATE_LIMITING_QUICK_REFERENCE.md and update it so it reads "TipTune uses environment variables for Redis and rate-limiting configuration." to improve consistency and readability.backend/WAVEFORM_ARCHITECTURE.md (1)
116-119: Add language specifier to fenced code block.The fenced code block should specify a language for proper syntax highlighting and linting compliance.
🎨 Proposed fix
Compare this ADR against: -- `backend/src/waveform/` - current canonical implementation -- Git history: `3d9aafc` (mount-waveform introduction), `037bb31` (consolidation) -- `backend/tsconfig.build.json` - contains legacy exclusion for `mount-waveform` +```text +- `backend/src/waveform/` - current canonical implementation +- Git history: `3d9aafc` (mount-waveform introduction), `037bb31` (consolidation) +- `backend/tsconfig.build.json` - contains legacy exclusion for `mount-waveform` +```🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/WAVEFORM_ARCHITECTURE.md` around lines 116 - 119, The fenced code block that lists "- `backend/src/waveform/` - current canonical implementation", the two git hashes and "- `backend/tsconfig.build.json` - contains legacy exclusion for `mount-waveform`" needs a language specifier for proper highlighting; update the trailing ``` to ```text (or another appropriate language) so the block is fenced as ```text and remains otherwise unchanged.frontend/src/services/tipHistorySource.ts (1)
240-249: Add an exhaustiveness fallback ingetTipsByType.The switch covers the three current
typevalues, but with nodefaultthe inferred return type still includesundefinedif a future type is added (or iftypeis widened upstream), which would surface as a runtime crash at the callerresult.itemson line 237. A simplenevercheck makes this future-proof:♻️ Suggested change
private async getTipsByType(type: 'sent' | 'received' | 'gifted', filters?: TipFiltersState, page = 1, pageSize = 10) { switch (type) { case 'sent': return this.getSentTips(filters, page, pageSize); case 'received': return this.getReceivedTips(filters, page, pageSize); case 'gifted': return this.getGiftedTips(filters, page, pageSize); + default: { + const _exhaustive: never = type; + return _exhaustive; + } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/services/tipHistorySource.ts` around lines 240 - 249, The switch in getTipsByType currently handles 'sent'|'received'|'gifted' but has no default, which lets the function possibly return undefined if the union is extended; add an exhaustive fallback (e.g., a default case that calls an assertNever or throws an Error mentioning the unexpected type) so getTipsByType always returns a value and surfaces unknown types immediately; update the function getTipsByType to include this default/exhaustiveness check to prevent callers from receiving undefined.frontend/src/__tests__/tipHistorySource.test.ts (1)
110-115: Dynamic import +anyhere is unnecessary.
vi.mockis hoisted, so a plain top-levelimport { tipService } from '../services/tipService'would already resolve to the mocked module (the same onetipHistorySource.tsconsumes), letting you drop both thelet tipService: anyand theawait import(...)and get proper typing onvi.mocked(tipService.getUserHistory).♻️ Suggested simplification
import { describe, it, expect, vi, beforeEach } from 'vitest'; import { FixtureTipHistorySource, ApiTipHistorySource } from '../services/tipHistorySource'; import { mockTipHistoryData } from '../fixtures/tipHistory.fixtures'; +import { tipService } from '../services/tipService'; import type { TipFiltersState } from '../components/tip-history'; @@ describe('ApiTipHistorySource', () => { - let tipService: any; - - beforeEach(async () => { + beforeEach(() => { vi.clearAllMocks(); - ({ tipService } = await import('../services/tipService')); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/__tests__/tipHistorySource.test.ts` around lines 110 - 115, Remove the unnecessary dynamic import and any-typed variable: replace the runtime import pattern in the test with a top-level ES import of tipService (import { tipService } from '../services/tipService') and delete the `let tipService: any` and the await import in beforeEach; keep vi.clearAllMocks() in beforeEach, and update usages to rely on the hoisted vi.mock so you can use vi.mocked(tipService.getUserHistory) for proper typing. Ensure imports match what tipHistorySource.ts consumes so the mocked module is identical to the one under test.frontend/src/__tests__/TipHistoryPage.test.tsx (1)
165-178: Spy is set up after the (unused) manualnew, which obscures intent.
new FixtureTipHistorySource()on line 167 runs before the spy on line 168 is installed, so the constructor invocation isn't tracked — the manually-created instance only matters becausemockImplementationthen returns it. The "tricky to mock properly" comment on line 171 also signals the test isn't really exercising the empty-state branch (it just renders default fixtures).Either delete this scaffolding (since the assertion on line 176 doesn't depend on it), or invert the order so the spy is installed first and the
mockSourceis constructed inside the factory. The same pattern repeats at lines 195–198 in the API source test.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/__tests__/TipHistoryPage.test.tsx` around lines 165 - 178, The test creates a FixtureTipHistorySource instance before installing the spy, so the constructor call isn't tracked; either remove the unused manual instantiation or install the spy first and have the spy's mockImplementation create and return the FixtureTipHistorySource instance, e.g. move the vi.spyOn(tipHistorySourceModule, 'FixtureTipHistorySource') call before creating mockSource and construct mockSource inside the mockImplementation; apply the same fix to the repeated pattern in the API source test (the block around lines 195–198) so the spy actually controls instantiation used by render(<TipHistoryPage />).frontend/src/hooks/__tests__/useToastQueue.test.ts (1)
6-10: Add explicitviimport for consistency with the majority of test files.Since
globals: trueis enabled in the Vitest config, the barevireferences (lines 7, 26, 82, 105, 124) work without error. However, most other test files in the codebase explicitly importvifrom'vitest'(e.g.,tipHistorySource.test.ts,TipHistoryPage.test.tsx, and 30+ others). For consistency, addvito the import statement at the top of the file:-import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/hooks/__tests__/useToastQueue.test.ts` around lines 6 - 10, Add an explicit import for vi from 'vitest' at the top of the test file so the bare vi references (used by calls like vi.useFakeTimers() and vi.useRealTimers(), and other vi.* usages in this file) match the project's convention; update the existing import statement (where other testing helpers are imported) to include vi so all vi calls in useToastQueue.test.ts resolve consistently with other tests.
🤖 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/README.md`:
- Line 118: The README documents a non-existent `GET
/api/v1/tracks/:trackId/waveform/status` endpoint; remove that line and instead
document that `GET /api/v1/tracks/:trackId/waveform` returns the waveform status
and peak data (via the controller method getStatus()), and keep `POST
/tracks/:trackId/waveform/regenerate` as the regeneration route to match the
controller implementation.
In `@backend/src/waveform/README.md`:
- Around line 5-6: The README.md in backend/src/waveform contains broken links
that use a 'backend/' prefix; update any references to
backend/WAVEFORM_ARCHITECTURE.md (and other links starting with 'backend/') to
the correct relative path from this file (use ../../WAVEFORM_ARCHITECTURE.md) so
links resolve to the top-level WAVEFORM_ARCHITECTURE.md; search for occurrences
in README.md and replace them with the relative path (README.md ->
../../WAVEFORM_ARCHITECTURE.md).
In `@backend/WAVEFORM_ARCHITECTURE.md`:
- Around line 100-102: The "Update documentation references" bullet in the
"Future Work" section is already completed in this PR; update the Future Work
entry by either removing the "Update documentation references" bullet entirely
or changing it to a completed note (e.g., "Completed in this PR: added waveform
module to backend/README.md and documented consolidation in
backend/src/waveform/README.md") so the doc no longer lists completed tasks as
pending; locate the "Future Work" header and the specific "Update documentation
references" bullet to make this change.
In `@contracts/auto-royalty-distribution/src/test.rs`:
- Around line 385-391: The test's ledger TTL ceiling is too small: update the
test setup where env.ledger().set_max_entry_ttl(...) is called so max_entry_ttl
>= PERSISTENT_BUMP_AMOUNT (recommend >= 600_000) or compute it from the storage
constant to keep in sync; specifically, adjust the value used in the test file
(referenced by set_max_entry_ttl) to match/derive from the
PERSISTENT_BUMP_AMOUNT defined in storage.rs so that calls from get_splits ->
extend_persistent won't exceed the configured TTL.
In `@docs/environment-reference.md`:
- Around line 22-27: The table lists DB env vars (DB_HOST, DB_PORT, DB_USERNAME,
DB_PASSWORD, DB_NAME) as "Required: Yes" but also shows concrete defaults, which
is contradictory; update the environment-reference.md table to use a consistent
model: either mark these vars as "Required: No" and note the default values
apply for local/development, or change the "Required" column to something like
"Required (production)" and add a new "Default (development)" column or inline
note indicating the provided default values are used in development; ensure the
rows for DB_HOST, DB_PORT, DB_USERNAME, DB_PASSWORD, and DB_NAME clearly state
when the default is applied and when the variable must be set (e.g., in
production).
In `@frontend/src/__tests__/TipHistoryPage.test.tsx`:
- Around line 192-203: The test's spy on
tipHistorySourceModule.ApiTipHistorySource can miss the actual constructor used
by TipHistoryPage because TipHistoryPage uses a direct import; replace the
namespace spy with a module-level mock of the ApiTipHistorySource export (mock
the module that exports ApiTipHistorySource) before importing/rendering
TipHistoryPage, returning your mockApiSource instance, then assert the mocked
ApiTipHistorySource was called with ('user123', undefined); reference
ApiTipHistorySource and TipHistoryPage to locate where to apply the mock.
In `@frontend/src/components/tip-history/TipFilters.tsx`:
- Around line 7-10: The optional TipFiltersState fields
(dateFrom/dateTo/amountMin/amountMax) can be undefined and thus leak into the
controlled inputs in the TipFilters component; fix this by ensuring every
input's value uses a defensive fallback (e.g. coalesce undefined to '') when
rendering (update the value= expressions inside TipFilters to use dateFrom ?? ''
/ dateTo ?? '' / amountMin ?? '' / amountMax ?? ''), or alternatively make those
fields non-optional in TipFiltersState and keep defaultTipFilters as the
canonical empty-string seed; update the input value bindings in TipFilters and
any handlers that construct partial TipFiltersState accordingly so React inputs
remain controlled.
In `@frontend/src/pages/TipHistoryPage.tsx`:
- Around line 91-101: The useEffect in TipHistoryPage that calls fetchStats
(which awaits tipHistorySource.getStats() then calls setStats) can race or set
state after unmount or after tipHistorySource swaps; add a local cancelled flag
(e.g., let cancelled = false) at the top of the effect, check cancelled before
calling setStats, and return a cleanup function that sets cancelled = true;
apply the same pattern inside the fetchCurrentTabData routine (the function that
awaits tab/page data and then calls setTabData or similar) so it checks the same
cancelled flag (or its own effect-scoped flag) before updating state to avoid
stale or unmounted updates.
- Around line 240-245: The code is using a hardcoded fixture id 'u1' to decide
gift direction which will break with real backend data; update the logic in the
component rendering the tip card (the lines setting variant and giftVariant that
check tip.gift?.recipient.id) to compare against the actual current user id
instead of 'u1' — obtain the id from the same auth/wallet context or prop used
elsewhere (e.g., useAuth().user.id or a currentUserId prop) and replace the
tip.gift?.recipient.id === 'u1' checks with tip.gift?.recipient.id ===
currentUserId so sent/received/given/received variants are computed correctly
based on the real user.
In `@frontend/src/services/tipHistorySource.ts`:
- Around line 188-226: Change the filter parameter types on ApiTipHistorySource
methods to match the TipHistorySource interface by accepting
Partial<TipFiltersState> (and defaulting to {}), e.g., update
getSentTips(filters: Partial<TipFiltersState> = {}, ...) and
getReceivedTips(filters: Partial<TipFiltersState> = {}, ...); likewise widen
getAllTipsForExport and getTipsByType to Partial<TipFiltersState> = {} so
callers and FixtureTipHistorySource are consistent, and rely on
applyFiltersAndSort (which already accepts Partials) to fill defaults/handle
missing fields.
---
Outside diff comments:
In `@backend/src/waveform/README.md`:
- Around line 93-131: Update the README endpoint paths to match the controller
routes: replace GET /api/waveform/:trackId with GET
/api/v1/tracks/:trackId/waveform, GET /api/waveform/:trackId/status with GET
/api/v1/tracks/:trackId/waveform/status, and POST
/api/waveform/:trackId/regenerate with POST
/api/v1/tracks/:trackId/waveform/regenerate; keep the existing response schemas
(id, trackId, waveformData, dataPoints, peakAmplitude, generationStatus,
processingDurationMs, createdAt, updatedAt and status/retryCount) unchanged and
ensure examples reflect the new paths so docs align with the controller routes
handling waveform generation.
In `@contracts/auto-royalty-distribution/src/queries.rs`:
- Around line 8-23: get_settlements_by_payout_id currently ignores its payout_id
and always returns an empty Vec, masking unimplemented behavior; change it to
return a clear error instead (e.g., add Error::NotImplemented to the Error enum
and return Err(Error::NotImplemented) from get_settlements_by_payout_id), update
AutoRoyaltyDistribution::get_settlements_by_payout to propagate that error
rather than treating an empty Vec as “no settlements”, and replace the scratch
comments with a proper doc comment on get_settlements_by_payout_id explaining
the missing implementation and intended behavior.
---
Nitpick comments:
In `@backend/RATE_LIMITING_QUICK_REFERENCE.md`:
- Line 13: The sentence "TipTune uses environment variables for Redis and rate
limiting configuration." needs a tiny wording polish: hyphenate "rate limiting
configuration" to "rate-limiting configuration"; locate the exact sentence in
RATE_LIMITING_QUICK_REFERENCE.md and update it so it reads "TipTune uses
environment variables for Redis and rate-limiting configuration." to improve
consistency and readability.
In `@backend/WAVEFORM_ARCHITECTURE.md`:
- Around line 116-119: The fenced code block that lists "-
`backend/src/waveform/` - current canonical implementation", the two git hashes
and "- `backend/tsconfig.build.json` - contains legacy exclusion for
`mount-waveform`" needs a language specifier for proper highlighting; update the
trailing ``` to ```text (or another appropriate language) so the block is fenced
as ```text and remains otherwise unchanged.
In `@docs/environment-reference.md`:
- Around line 31-34: Update the environment reference table entries for
JWT_SECRET and EMBED_SECRET to explicitly mark their example values as
development-only fallbacks: change the description or add a parenthetical to the
variable names/descriptions (e.g., "JWT_SECRET (dev-only fallback)" and
"EMBED_SECRET (dev-only fallback)") and update the example values to indicate
they are unsafe for production (e.g., `dev-jwt-secret-placeholder` /
`dev-embed-secret-placeholder`), plus add a short inline note below the table
clarifying that these examples must be overridden with secure secrets in
production. Ensure the edits touch the `JWT_SECRET` and `EMBED_SECRET` rows and
the table caption/footnote in environment-reference.md.
In `@frontend/src/__tests__/TipHistoryPage.test.tsx`:
- Around line 165-178: The test creates a FixtureTipHistorySource instance
before installing the spy, so the constructor call isn't tracked; either remove
the unused manual instantiation or install the spy first and have the spy's
mockImplementation create and return the FixtureTipHistorySource instance, e.g.
move the vi.spyOn(tipHistorySourceModule, 'FixtureTipHistorySource') call before
creating mockSource and construct mockSource inside the mockImplementation;
apply the same fix to the repeated pattern in the API source test (the block
around lines 195–198) so the spy actually controls instantiation used by
render(<TipHistoryPage />).
In `@frontend/src/__tests__/tipHistorySource.test.ts`:
- Around line 110-115: Remove the unnecessary dynamic import and any-typed
variable: replace the runtime import pattern in the test with a top-level ES
import of tipService (import { tipService } from '../services/tipService') and
delete the `let tipService: any` and the await import in beforeEach; keep
vi.clearAllMocks() in beforeEach, and update usages to rely on the hoisted
vi.mock so you can use vi.mocked(tipService.getUserHistory) for proper typing.
Ensure imports match what tipHistorySource.ts consumes so the mocked module is
identical to the one under test.
In `@frontend/src/hooks/__tests__/useToastQueue.test.ts`:
- Around line 6-10: Add an explicit import for vi from 'vitest' at the top of
the test file so the bare vi references (used by calls like vi.useFakeTimers()
and vi.useRealTimers(), and other vi.* usages in this file) match the project's
convention; update the existing import statement (where other testing helpers
are imported) to include vi so all vi calls in useToastQueue.test.ts resolve
consistently with other tests.
In `@frontend/src/services/tipHistorySource.ts`:
- Around line 240-249: The switch in getTipsByType currently handles
'sent'|'received'|'gifted' but has no default, which lets the function possibly
return undefined if the union is extended; add an exhaustive fallback (e.g., a
default case that calls an assertNever or throws an Error mentioning the
unexpected type) so getTipsByType always returns a value and surfaces unknown
types immediately; update the function getTipsByType to include this
default/exhaustiveness check to prevent callers from receiving undefined.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1815083d-6247-4148-ad7d-9c13361986a7
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (22)
README.mdbackend/IMPLEMENTATION_SUMMARY.mdbackend/RATE_LIMITING_QUICK_REFERENCE.mdbackend/README.mdbackend/WAVEFORM_ARCHITECTURE.mdbackend/dev-onboarding.mdbackend/src/waveform/README.mdcontracts/README.mdcontracts/auto-royalty-distribution/src/queries.rscontracts/auto-royalty-distribution/src/test.rsdocs/environment-reference.mdfrontend/README.mdfrontend/src/App.tsxfrontend/src/__tests__/TipHistoryPage.test.tsxfrontend/src/__tests__/tipHistorySource.test.tsfrontend/src/components/SearchModule.tsxfrontend/src/components/tip-history/TipFilters.tsxfrontend/src/hooks/__tests__/useToastQueue.test.tsfrontend/src/pages/TipHistoryPage.tsxfrontend/src/services/tipHistorySource.tsfrontend/src/services/tipService.tsfrontend/tsconfig.json
💤 Files with no reviewable changes (1)
- frontend/src/services/tipService.ts
|
|
||
| - `GET /api/v1/tracks/:trackId/waveform` - Get waveform data for a track | ||
| - `POST /api/v1/tracks/:trackId/waveform/regenerate` - Trigger waveform regeneration | ||
| - `GET /api/v1/tracks/:trackId/waveform/status` - Get waveform generation status |
There was a problem hiding this comment.
The /status endpoint does not exist in the controller.
Based on the controller implementation (context snippet 1), there are only two routes:
GET /tracks/:trackId/waveform- returns status and peak data (viagetStatus())POST /tracks/:trackId/waveform/regenerate- triggers regeneration
The GET /tracks/:trackId/waveform endpoint already returns WaveformStatusDto, so line 118 should be removed as it documents a non-existent separate status endpoint.
📝 Proposed fix
- `GET /api/v1/tracks/:trackId/waveform` - Get waveform data for a track
- `POST /api/v1/tracks/:trackId/waveform/regenerate` - Trigger waveform regeneration
-- `GET /api/v1/tracks/:trackId/waveform/status` - Get waveform generation status📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - `GET /api/v1/tracks/:trackId/waveform/status` - Get waveform generation status | |
| - `GET /api/v1/tracks/:trackId/waveform` - Get waveform data for a track | |
| - `POST /api/v1/tracks/:trackId/waveform/regenerate` - Trigger waveform regeneration |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/README.md` at line 118, The README documents a non-existent `GET
/api/v1/tracks/:trackId/waveform/status` endpoint; remove that line and instead
document that `GET /api/v1/tracks/:trackId/waveform` returns the waveform status
and peak data (via the controller method getStatus()), and keep `POST
/tracks/:trackId/waveform/regenerate` as the regeneration route to match the
controller implementation.
Comment on lines
+5
to
+6
| > **Note**: This is the canonical waveform implementation. A duplicate module `mount-waveform` existed temporarily during a refactor and was consolidated into this module in April 2026. See `backend/WAVEFORM_ARCHITECTURE.md` for the full history and architecture decision record. | ||
|
|
There was a problem hiding this comment.
Fix broken relative links to architecture docs.
From backend/src/waveform/README.md, links prefixed with backend/ resolve to a non-existent nested path. Use relative paths from this directory.
🔧 Proposed doc-link fix
-> **Note**: This is the canonical waveform implementation. A duplicate module `mount-waveform` existed temporarily during a refactor and was consolidated into this module in April 2026. See `backend/WAVEFORM_ARCHITECTURE.md` for the full history and architecture decision record.
+> **Note**: This is the canonical waveform implementation. A duplicate module `mount-waveform` existed temporarily during a refactor and was consolidated into this module in April 2026. See `../../WAVEFORM_ARCHITECTURE.md` for the full history and architecture decision record.
...
-- **`backend/WAVEFORM_ARCHITECTURE.md`** - Full ADR with decision record, component diagram, and cleanup tasks
-- **`backend/waveform-architecture.md`** - Existing implementation guide (legacy filename)
+- **`../../WAVEFORM_ARCHITECTURE.md`** - Full ADR with decision record, component diagram, and cleanup tasks
+- **`../../waveform-architecture.md`** - Existing implementation guide (legacy filename)Also applies to: 195-199
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/waveform/README.md` around lines 5 - 6, The README.md in
backend/src/waveform contains broken links that use a 'backend/' prefix; update
any references to backend/WAVEFORM_ARCHITECTURE.md (and other links starting
with 'backend/') to the correct relative path from this file (use
../../WAVEFORM_ARCHITECTURE.md) so links resolve to the top-level
WAVEFORM_ARCHITECTURE.md; search for occurrences in README.md and replace them
with the relative path (README.md -> ../../WAVEFORM_ARCHITECTURE.md).
Comment on lines
+100
to
+102
| 2. **Update documentation references**: | ||
| - `backend/README.md` - add Waveform to module list | ||
| - `backend/src/waveform/README.md` - note consolidation status |
There was a problem hiding this comment.
Cleanup task 2 is already completed by this PR.
The "Update documentation references" task lists:
backend/README.md- add Waveform to module listbackend/src/waveform/README.md- note consolidation status
However, both of these updates are already included in this PR:
- Lines 166-175 of
backend/README.mdadd the waveform module structure - Context snippet 2 shows
backend/src/waveform/README.mdalready documents the consolidation
Consider either removing this task from the "Future Work" section or updating it to reflect that it's been completed as part of this PR.
📝 Proposed fix
-2. **Update documentation references**:
- - `backend/README.md` - add Waveform to module list
- - `backend/src/waveform/README.md` - note consolidation status
+2. **Update documentation references**: ✅ Completed in this PR
+ - `backend/README.md` - waveform module added to structure section
+ - `backend/src/waveform/README.md` - consolidation status documented🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/WAVEFORM_ARCHITECTURE.md` around lines 100 - 102, The "Update
documentation references" bullet in the "Future Work" section is already
completed in this PR; update the Future Work entry by either removing the
"Update documentation references" bullet entirely or changing it to a completed
note (e.g., "Completed in this PR: added waveform module to backend/README.md
and documented consolidation in backend/src/waveform/README.md") so the doc no
longer lists completed tasks as pending; locate the "Future Work" header and the
specific "Update documentation references" bullet to make this change.
Comment on lines
+385
to
+391
| env.ledger().set_sequence_number(100); | ||
| env.ledger().set_timestamp(1000); | ||
| env.ledger().set_network_id([0; 32]); | ||
| env.ledger().set_base_reserve(100); | ||
| env.ledger().set_min_persistent_entry_ttl(10); | ||
| env.ledger().set_min_temp_entry_ttl(10); | ||
| env.ledger().set_max_entry_ttl(100_000); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find PERSISTENT_BUMP_AMOUNT (and friends) so we can compare to the test's max_entry_ttl=100_000.
rg -nP --type=rust -C2 '\b(PERSISTENT_BUMP_AMOUNT|PERSISTENT_THRESHOLD|TEMP_BUMP_AMOUNT|DAY_IN_LEDGERS)\s*[:=]'Repository: OlufunbiIK/tip-tune
Length of output: 789
Fix test ledger TTL configuration—max_entry_ttl is far too small for PERSISTENT_BUMP_AMOUNT.
The test sets set_max_entry_ttl(100_000), but PERSISTENT_BUMP_AMOUNT in storage.rs is 30 * DAY_IN_LEDGERS = 30 * 17280 = 518,400. When get_splits triggers extend_persistent with a bump of 518,400, the TTL extension will exceed the configured maximum of 100,000, causing the host to panic with a TTL-bound error. Set max_entry_ttl to at least 600,000, or better yet, derive it from the storage constants to keep them in sync.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@contracts/auto-royalty-distribution/src/test.rs` around lines 385 - 391, The
test's ledger TTL ceiling is too small: update the test setup where
env.ledger().set_max_entry_ttl(...) is called so max_entry_ttl >=
PERSISTENT_BUMP_AMOUNT (recommend >= 600_000) or compute it from the storage
constant to keep in sync; specifically, adjust the value used in the test file
(referenced by set_max_entry_ttl) to match/derive from the
PERSISTENT_BUMP_AMOUNT defined in storage.rs so that calls from get_splits ->
extend_persistent won't exceed the configured TTL.
Comment on lines
+22
to
+27
| | `DB_HOST` | Database host address | **Yes** | `localhost` | | ||
| | `DB_PORT` | Database port | **Yes** | `5432` | | ||
| | `DB_USERNAME` | Database user | **Yes** | `postgres` | | ||
| | `DB_PASSWORD` | Database password | **Yes** | `password` | | ||
| | `DB_NAME` | Database name | **Yes** | `tiptune` | | ||
|
|
There was a problem hiding this comment.
Clarify “Required” vs “Default” for DB variables.
These rows currently mark DB_HOST, DB_PORT, DB_USERNAME, DB_PASSWORD, and DB_NAME as Required: Yes while also listing concrete defaults. That is contradictory for readers setting up .env.
Use one model consistently (e.g., “Required in production” + “default used in development”), or mark these as not required when defaults are applied.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/environment-reference.md` around lines 22 - 27, The table lists DB env
vars (DB_HOST, DB_PORT, DB_USERNAME, DB_PASSWORD, DB_NAME) as "Required: Yes"
but also shows concrete defaults, which is contradictory; update the
environment-reference.md table to use a consistent model: either mark these vars
as "Required: No" and note the default values apply for local/development, or
change the "Required" column to something like "Required (production)" and add a
new "Default (development)" column or inline note indicating the provided
default values are used in development; ensure the rows for DB_HOST, DB_PORT,
DB_USERNAME, DB_PASSWORD, and DB_NAME clearly state when the default is applied
and when the variable must be set (e.g., in production).
Comment on lines
192
to
203
| it('should use API source when user/artist ID is provided', () => { | ||
| vi.stubEnv('VITE_DEV_USER_ID', 'user123'); | ||
|
|
||
| const mockApiSource = new tipService.ApiTipHistorySource('user123'); | ||
| vi.spyOn(tipService, 'ApiTipHistorySource').mockReturnValue(mockApiSource); | ||
| const mockApiSource = new ApiTipHistorySource('user123'); | ||
| vi.spyOn(tipHistorySourceModule, 'ApiTipHistorySource').mockImplementation(function() { | ||
| return mockApiSource; | ||
| }); | ||
|
|
||
| render(<TipHistoryPage />); | ||
|
|
||
| expect(tipService.ApiTipHistorySource).toHaveBeenCalledWith('user123', undefined); | ||
| expect(tipHistorySourceModule.ApiTipHistorySource).toHaveBeenCalledWith('user123', undefined); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm spyOn on the module is the only signal in this assertion path.
rg -nP "spyOn\([^)]*tipHistorySourceModule" --type=ts --type=tsx
rg -nP "ApiTipHistorySource\s*\(" frontend/src/pages/TipHistoryPage.tsx
# Also verify Vitest version supports module-namespace spy
fd -t f 'package.json' --exec rg -n '"vitest"' {}Repository: OlufunbiIK/tip-tune
Length of output: 221
🏁 Script executed:
# Check the test file imports and full test context
cat -n frontend/src/__tests__/TipHistoryPage.test.tsx | head -30
cat -n frontend/src/__tests__/TipHistoryPage.test.tsx | sed -n '185,210p'
# Check the production file imports and how it uses ApiTipHistorySource
cat -n frontend/src/pages/TipHistoryPage.tsx | head -30Repository: OlufunbiIK/tip-tune
Length of output: 3802
🏁 Script executed:
# Search for other uses of ApiTipHistorySource in tests
rg -n "ApiTipHistorySource" frontend/src/__tests__/TipHistoryPage.test.tsx -A 2 -B 2
# Check if there's a separate test for useTipHistorySource hook
rg -n "useTipHistorySource" frontend/src --type ts
# Look for any other spyOn patterns in the test file
rg -n "spyOn" frontend/src/__tests__/TipHistoryPage.test.tsxRepository: OlufunbiIK/tip-tune
Length of output: 1228
The spy on namespace-imported class may not intercept calls from the direct import used in production code.
The test spies on tipHistorySourceModule.ApiTipHistorySource (namespace reference), but production code at line 24 of TipHistoryPage.tsx imports and calls new ApiTipHistorySource(userId || undefined, artistId || undefined) using the direct import. Depending on how Vite/Vitest handles module binding in your setup, these may not be the same reference, causing the spy to miss the call entirely. The assertion would then be ineffective—the test would pass even if the production code stopped calling ApiTipHistorySource.
Consider spying on the direct import instead:
vi.spyOn(ApiTipHistorySource.prototype, 'constructor')or restructure to mock at the module level before imports if the direct-import approach doesn't work.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/__tests__/TipHistoryPage.test.tsx` around lines 192 - 203, The
test's spy on tipHistorySourceModule.ApiTipHistorySource can miss the actual
constructor used by TipHistoryPage because TipHistoryPage uses a direct import;
replace the namespace spy with a module-level mock of the ApiTipHistorySource
export (mock the module that exports ApiTipHistorySource) before
importing/rendering TipHistoryPage, returning your mockApiSource instance, then
assert the mocked ApiTipHistorySource was called with ('user123', undefined);
reference ApiTipHistorySource and TipHistoryPage to locate where to apply the
mock.
Comment on lines
+7
to
+10
| dateFrom?: string; | ||
| dateTo?: string; | ||
| amountMin?: string; | ||
| amountMax?: string; |
There was a problem hiding this comment.
Optional fields can leak undefined into controlled inputs.
Now that dateFrom/dateTo/amountMin/amountMax are string | undefined, any code path that constructs a TipFiltersState without these fields will pass undefined to the value= prop on lines 61, 69, 84, 95, switching them from controlled to uncontrolled (React warning + potential value loss after first user interaction). defaultTipFilters still seeds them with '', so today's flow is fine, but the type now allows the regression.
Defensive fallback at the input layer keeps the component robust to partial inputs:
🛡️ Optional defensive fix
- value={filters.dateFrom}
+ value={filters.dateFrom ?? ''}
@@
- value={filters.dateTo}
+ value={filters.dateTo ?? ''}
@@
- value={filters.amountMin}
+ value={filters.amountMin ?? ''}
@@
- value={filters.amountMax}
+ value={filters.amountMax ?? ''}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/tip-history/TipFilters.tsx` around lines 7 - 10, The
optional TipFiltersState fields (dateFrom/dateTo/amountMin/amountMax) can be
undefined and thus leak into the controlled inputs in the TipFilters component;
fix this by ensuring every input's value uses a defensive fallback (e.g.
coalesce undefined to '') when rendering (update the value= expressions inside
TipFilters to use dateFrom ?? '' / dateTo ?? '' / amountMin ?? '' / amountMax ??
''), or alternatively make those fields non-optional in TipFiltersState and keep
defaultTipFilters as the canonical empty-string seed; update the input value
bindings in TipFilters and any handlers that construct partial TipFiltersState
accordingly so React inputs remain controlled.
Comment on lines
91
to
+101
| useEffect(() => { | ||
| setPage(1); | ||
| }, [activeTab, filters]); | ||
|
|
||
| const stats = useMemo(() => tipHistorySource.getStats(), [tipHistorySource]); | ||
| const fetchStats = async () => { | ||
| try { | ||
| const s = await tipHistorySource.getStats(); | ||
| setStats(s); | ||
| } catch (e) { | ||
| console.error('Failed to fetch stats:', e); | ||
| } | ||
| }; | ||
| fetchStats(); | ||
| }, [tipHistorySource]); |
There was a problem hiding this comment.
Stats effect can race and setState on unmount.
If tipHistorySource swaps (e.g., env-driven user/artist id changes during dev hot-reload, or fast-refresh) or the component unmounts before getStats() resolves, the late setStats(s) either overwrites the newer source's stats or warns about unmounted state updates. Add a cancelled flag in the effect.
🛡️ Suggested cancellation
useEffect(() => {
+ let cancelled = false;
const fetchStats = async () => {
try {
const s = await tipHistorySource.getStats();
- setStats(s);
+ if (!cancelled) setStats(s);
} catch (e) {
console.error('Failed to fetch stats:', e);
}
};
fetchStats();
+ return () => {
+ cancelled = true;
+ };
}, [tipHistorySource]);The same pattern would also benefit fetchCurrentTabData (lines 49–79) where stale results from a previous tab/page can land after a faster newer request.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/pages/TipHistoryPage.tsx` around lines 91 - 101, The useEffect
in TipHistoryPage that calls fetchStats (which awaits
tipHistorySource.getStats() then calls setStats) can race or set state after
unmount or after tipHistorySource swaps; add a local cancelled flag (e.g., let
cancelled = false) at the top of the effect, check cancelled before calling
setStats, and return a cleanup function that sets cancelled = true; apply the
same pattern inside the fetchCurrentTabData routine (the function that awaits
tab/page data and then calls setTabData or similar) so it checks the same
cancelled flag (or its own effect-scoped flag) before updating state to avoid
stale or unmounted updates.
Comment on lines
+240
to
245
| variant={activeTab === 'gifted' ? (tip.gift?.recipient.id === 'u1' ? 'received' : 'sent') : activeTab as 'sent' | 'received'} | ||
| giftVariant={ | ||
| activeTab === 'gifts' | ||
| activeTab === 'gifted' | ||
| ? tip.gift?.recipient.id === 'u1' ? 'received' : 'given' | ||
| : undefined | ||
| } |
There was a problem hiding this comment.
Hardcoded 'u1' recipient check will misclassify gifted tips for real users.
tip.gift?.recipient.id === 'u1' is a fixture-data sentinel for "this user". Once ApiTipHistorySource starts returning real backend data, every gifted tip will fall to the 'sent'/'given' branch regardless of whether the current user actually received the gift, flipping the card label and giftVariant styling.
Wire this through the real current-user id (e.g., from wallet/auth context or a prop) — the same source the fixtures already model with 'u1'.
🐛 Sketch of the fix
- variant={activeTab === 'gifted' ? (tip.gift?.recipient.id === 'u1' ? 'received' : 'sent') : activeTab as 'sent' | 'received'}
+ variant={
+ activeTab === 'gifted'
+ ? (tip.gift?.recipient.id === currentUserId ? 'received' : 'sent')
+ : (activeTab as 'sent' | 'received')
+ }
giftVariant={
activeTab === 'gifted'
- ? tip.gift?.recipient.id === 'u1' ? 'received' : 'given'
+ ? (tip.gift?.recipient.id === currentUserId ? 'received' : 'given')
: undefined
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/pages/TipHistoryPage.tsx` around lines 240 - 245, The code is
using a hardcoded fixture id 'u1' to decide gift direction which will break with
real backend data; update the logic in the component rendering the tip card (the
lines setting variant and giftVariant that check tip.gift?.recipient.id) to
compare against the actual current user id instead of 'u1' — obtain the id from
the same auth/wallet context or prop used elsewhere (e.g., useAuth().user.id or
a currentUserId prop) and replace the tip.gift?.recipient.id === 'u1' checks
with tip.gift?.recipient.id === currentUserId so sent/received/given/received
variants are computed correctly based on the real user.
Comment on lines
+188
to
+226
| async getSentTips(filters: TipFiltersState = { sort: 'newest', assetType: 'all', searchQuery: '' }, page = 1, pageSize = 10) { | ||
| if (!this.userId) { | ||
| return { items: [], total: 0, hasMore: false }; | ||
| } | ||
|
|
||
| try { | ||
| const response = await tipService.getUserHistory(this.userId, page, pageSize); | ||
| const items = response.data?.map(this.mapApiTipToHistoryItem) ?? []; | ||
| const filtered = applyFiltersAndSort(items, filters); | ||
| return { | ||
| items: filtered, | ||
| total: response.meta?.total ?? 0, | ||
| hasMore: response.meta?.hasNextPage ?? false, | ||
| }; | ||
| } catch (error) { | ||
| console.error('Failed to fetch sent tips:', error); | ||
| return { items: [], total: 0, hasMore: false }; | ||
| } | ||
| } | ||
|
|
||
| async getReceivedTips(filters: TipFiltersState = { sort: 'newest', assetType: 'all', searchQuery: '' }, page = 1, pageSize = 10) { | ||
| if (!this.artistId) { | ||
| return { items: [], total: 0, hasMore: false }; | ||
| } | ||
|
|
||
| try { | ||
| const response = await tipService.getArtistReceived(this.artistId, page, pageSize); | ||
| const items = response.data?.map(this.mapApiTipToHistoryItem) ?? []; | ||
| const filtered = applyFiltersAndSort(items, filters); | ||
| return { | ||
| items: filtered, | ||
| total: response.meta?.total ?? 0, | ||
| hasMore: response.meta?.hasNextPage ?? false, | ||
| }; | ||
| } catch (error) { | ||
| console.error('Failed to fetch received tips:', error); | ||
| return { items: [], total: 0, hasMore: false }; | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Param types diverge from the TipHistorySource interface (and from FixtureTipHistorySource).
The interface declares filters?: Partial<TipFiltersState> (lines 14/23/32) and FixtureTipHistorySource matches it with Partial<TipFiltersState> = {}, but ApiTipHistorySource.getSentTips/getReceivedTips declare filters: TipFiltersState = { sort: 'newest', assetType: 'all', searchQuery: '' }. That:
- forces the implementation to manufacture a fake "full" default that no longer reflects
defaultTipFilters(e.g., missingdateFrom/amountMin/etc.), so the two sources can't be reasoned about uniformly, - is stricter than the interface — a caller passing a true
Partial<TipFiltersState>through the interface compiles, but constructing anApiTipHistorySourceand calling it directly will not.
Align with the interface and let applyFiltersAndSort (which already handles Partial) do the work:
♻️ Recommended fix
- async getSentTips(filters: TipFiltersState = { sort: 'newest', assetType: 'all', searchQuery: '' }, page = 1, pageSize = 10) {
+ async getSentTips(filters: Partial<TipFiltersState> = {}, page = 1, pageSize = 10) {
@@
- async getReceivedTips(filters: TipFiltersState = { sort: 'newest', assetType: 'all', searchQuery: '' }, page = 1, pageSize = 10) {
+ async getReceivedTips(filters: Partial<TipFiltersState> = {}, page = 1, pageSize = 10) {For consistency, getAllTipsForExport/getTipsByType parameters can be widened similarly (Partial<TipFiltersState>), since their downstream applyFiltersAndSort accepts it.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/services/tipHistorySource.ts` around lines 188 - 226, Change the
filter parameter types on ApiTipHistorySource methods to match the
TipHistorySource interface by accepting Partial<TipFiltersState> (and defaulting
to {}), e.g., update getSentTips(filters: Partial<TipFiltersState> = {}, ...)
and getReceivedTips(filters: Partial<TipFiltersState> = {}, ...); likewise widen
getAllTipsForExport and getTipsByType to Partial<TipFiltersState> = {} so
callers and FixtureTipHistorySource are consistent, and rely on
applyFiltersAndSort (which already accepts Partials) to fill defaults/handle
missing fields.
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.
I'll analyze the waveform module situation, create the ADR, and handle the branch workflow. Let me start by examining the relevant files.
closes #479
Summary by CodeRabbit
New Features
Documentation
Improvements