Release: develop -> main - #4489
Merged
Merged
Conversation
…ket counts (#4487) * fix(ledger): drop unreachable fallbacks on the mark-to-market counts legCount and valuedCount come from COUNT(*) aggregates without GROUP BY, so getRawOne always returns a row and neither column can be null. The `?? 0` guards could therefore never fire, and no test could reach them. Narrow the raw row type to match what the query actually returns and drop both guards; the SUM-based native and chf fallbacks stay, since those really can be null on an empty leg set. With the two unreachable branches gone the file reaches full coverage, so pin it in the ratchet. * docs(coverage): record the state after pinning the mark-to-market service 422 files pinned, totals refreshed against the commit they were measured on. The paragraph on unreachable branches no longer points at ledger-mark-to-market as an open case - both of its examples have since been closed by deleting the fallback. * docs(coverage): state that the numbers include this PR's change The gate run behind them was made on the branch, not on plain develop.
github-actions
Bot
requested review from
TaprootFreak and
davidleomay
as code owners
July 30, 2026 10:04
…s from the review (#4491) * test(log): fix wrong btcPriceChf attribution in F20b comment The comment claimed the null-to-0 default for btcPriceChf happened in mapSummaryToEntry and pointed at the wrong service spec line. The default is actually applied entirely inside getFinancialLogSummaries itself, so this repository-level test already covers it; only the other number fields are defaulted at the service call site. * docs: correct four overstated claims from the second review round - buy_fiat and buy_crypto do not filter on their own `created` column. Both use `transaction: { created: MoreThan(from) }`, i.e. the joined transaction table's column and `>` rather than `>=`, so an index on their own `created` would not serve that path at all. crypto_input is the one that filters its own column, via PayInService.getPayInFee. - `loops=3` means leader plus two workers, not three workers: max_parallel_workers_per_gather is 2 on this instance. - "clearly within the range where an index scan beats a sequential scan" claimed more certainty than the same docstring grants two paragraphs above, where it states the planner's choice was not verified in production. - The mapSummaryToEntry comment folded two different deviations under one rationale. Only the `null` entry is about favouring a visible gap over a dropped row; the wrongly-typed property is about honouring the `number | undefined` DTO contract. "still fails loud" also implied continuity where the behaviour deliberately changed. * revert: restore the already-merged trading-order index migration CONTRIBUTING.md: "Never edit a migration after merge to DEV — add a follow-up migration instead". 1785470000000-AddTradingOrderCreatedIndex.js shipped with #4484, so the three docstring corrections in the previous commit were not allowed there and failed the migration immutability check. The corrections themselves stand and are not lost: they concern claims about buy_fiat/buy_crypto filtering on transaction.created rather than their own column, about "loops=3" meaning leader plus two workers, and about an overstated certainty on the planner's index choice. They belong in the follow-up migration added by this PR, which has not been merged yet. The mapSummaryToEntry comment fix is unaffected and remains in place — that file is not a migration.
…t-loop delay (#4346) * perf(gs): queue sheet exports, cap unbounded queries and monitor event-loop delay * fix(gs): free wedged queue slots, shed timed-out exports, warn on default cap, consistent 400 on custom exports * fix(gs): detect default-cap truncation per bank_tx sub-query to avoid false-positive warns * fix(gs): use limit instead of take on joined bank_tx raw exports (take emits no LIMIT with joins) * fix(gs): drop duplicate imports introduced by the develop rebase The rebase onto develop combined two non-overlapping import additions for the same identifiers without git flagging a conflict: develop had added its own DbQueryDto/UserRole imports to this spec while the branch added a second set. tsc reported four TS2300 "Duplicate identifier" errors, so the suite could not compile. Merge DbReturnData into the existing absolute-path DbQueryDto import and drop the duplicate relative-path and UserRole lines. * fix(gs): address review findings on the export queue - queue-handler: defer the action call into the promise chain. A synchronous throw previously escaped doWork without ever calling reject, leaving the queue item unsettled and hanging the caller until the queue timeout. - monitor-event-loop: implement OnModuleDestroy and disable the histogram, so its 20ms sampling timer stops on teardown instead of running on. - gs.service: route identifier and table through Util.sanitizeLogValue in warnIfCapped; client-controlled values must never land raw in a log line. - gs.service: replace any[] with Record<string, unknown>[] in the getExtendedBankTxData return type per the no-any rule. * test(gs): cover the synchronous-throw regression in the export queue Commit 1547091 deferred the action call in QueueItem.doWork so a synchronous throw rejects the item instead of leaving it unsettled. Add the regression test that pins that behaviour: a synchronously throwing action must reject with its own error (not with a queue timeout), and the worker slot must be free for the next item afterwards. Also switch the spec to the absolute import path and move the QueueHandler import into the src/shared group, per CONTRIBUTING. --------- Co-authored-by: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com>
TaprootFreak
approved these changes
Jul 30, 2026
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.
Automatic Release PR
This PR was automatically created after changes were pushed to develop.
Commits: 1 new commit(s)
Checklist