Add Scalable Capital#304
Conversation
WalkthroughThis pull request adds support for Scalable Capital broker as a new converter module. The implementation includes version bump (0.34.0 → 0.35.0), a new ScalableCapitalConverter class that parses semicolon-delimited CSV files with comma decimal separators, maps asset types and transaction types to Ghostfolio activity formats, resolves securities via SecurityService using ISIN, handles parsing errors and ignored records (withdrawals/deposits), and provides progress logging. Supporting additions include the ScalableCapitalRecord model class, comprehensive unit tests covering success and error scenarios, integration into the converter factory, header mapping registration in the watcher, and README documentation updates listing Scalable Capital as a supported broker. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Analysis NotesThe changes span seven files with a mix of configuration, documentation, new implementation, and test coverage. The heterogeneous nature—combining model definition, converter logic with CSV parsing and type mapping, comprehensive tests, and integration points—requires separate reasoning for each component. However, the implementation follows established architectural patterns (extending AbstractConverter, using existing SecurityService integration, applying standard error handling conventions), which moderates the review complexity. Key review areas include CSV parsing correctness with semicolon delimiters and comma decimal handling, accuracy of Scalable Capital type mappings to Ghostfolio actions, security resolution logic, and completeness of test coverage for success and error paths. 🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (1 warning, 2 inconclusive)
✅ Passed checks (2 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 |
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
8-8:⚠️ Potential issue | 🟡 MinorBroker count is stale — should be 27.
With Scalable Capital added, the count should be updated from 26 to 27.
-This tool allows you to convert CSV transaction exports to an import file that can be read by [Ghostfolio](https://github.com/ghostfolio/ghostfolio/). Currently there is support for 26 brokers: +This tool allows you to convert CSV transaction exports to an import file that can be read by [Ghostfolio](https://github.com/ghostfolio/ghostfolio/). Currently there is support for 27 brokers:
🤖 Fix all issues with AI agents
In `@README.md`:
- Around line 166-168: The "Scalable Capital" README section currently contains
only a placeholder; replace the "to come" under the "### Scalable Capital"
heading with full export instructions consistent with other broker sections
(copy the structure and content pattern used by other broker entries: required
credentials, API endpoints, step-by-step export commands or UI steps, expected
file formats, and any environment variables), ensure you reference the exact
heading "### Scalable Capital" so the content sits in the correct place, and
update the PR checklist entry for README completion if present.
- Line 316: Update the table row for "Scalable Capital" to match the existing
alias format by changing the alias fragment from "or (`sc`)" to "(or `sc`)";
locate the row containing the command text `run start scalablecapital` and the
alias `sc` and make the parentheses placement consistent with other entries
(i.e., use "(or `sc`)").
In `@src/converters/scalableCapitalConverter.test.ts`:
- Line 27: Typo in the test comment: change the misspelled "Arange" to "Arrange"
in the scalableCapitalConverter.test.ts test file (locate the comment inside the
relevant test/describe block where "Arange" appears) so the test comments read
"Arrange" for consistency and clarity.
- Around line 148-168: The test currently uses header-only input and hits the
early-return errorCallback path instead of the converter's catch block; change
the test to supply content with at least one data row (so processFileContents
continues past the header) and then force an exception during processing by
stubbing a method invoked inside processFileContents (for example, spyOn the
ScalableCapitalConverter instance's internal processing helper like processRow
or the specific private method used to convert a row and make it throw). Call
sut.processFileContents with the same success and error callbacks and assert the
consoleSpy sees the "[e] An error occurred..." message and the stack trace in
the error callback to exercise the catch block (reference
ScalableCapitalConverter.processFileContents and the converter's internal
row-processing helper).
In `@src/converters/scalableCapitalConverter.ts`:
- Around line 119-134: The activity construction in scalableCapitalConverter.ts
currently sets fee to record.fee and omits tax; update the fee field in the
result.activities.push object to include both tax and fee as absolute values
(e.g., fee: Math.abs(record.tax || 0) + Math.abs(record.fee || 0)) so taxes are
not dropped—make this change inside the block that builds the activity (the
object pushed into result.activities) where accountId, comment, fee, quantity,
etc. are set.
- Around line 46-57: The number-parsing branch in scalableCapitalConverter (the
if checking context.column === "shares" || "price" || "amount" || "fee" ||
"tax") mishandles numbers with period thousands separators and comma decimals;
update the transformation before Number.parseFloat to first strip all periods
(thousands separators) and then replace the comma with a dot (e.g., use a global
remove of "." then replace "," with ".") so "-5.743,51" becomes "-5743.51"
before parsing; keep the existing empty-string check and
Math.abs(Number.parseFloat(...)) behavior unchanged.
- Line 119: The call to dayjs with a format string in the expression that
assigns const date = dayjs(`${record.date} ${record.time}`, "YYYY-MM-DD
HH:mm:ss") requires the customParseFormat plugin to be loaded; add an import for
customParseFormat from 'dayjs/plugin/customParseFormat' and call
dayjs.extend(customParseFormat) at module top (alongside other dayjs
imports/extensions) so the provided format string is respected by the parser.
- Around line 32-44: Update the fee mapping to combine tax and fee by replacing
the single use of record.fee with the sum Math.abs(record.fee) +
Math.abs(record.tax) so transaction costs include both fields (look for the code
that builds the output object using record.fee/record.tax). Also import and
register dayjs's customParseFormat plugin (add import of customParseFormat and
call dayjs.extend(customParseFormat)) so the explicit date format argument used
with dayjs(...) is honored.
In `@src/models/scalableCapitalRecord.ts`:
- Around line 1-15: The ScalableCapitalRecord.date property is declared as Date
but CSV parsing leaves it as a string (and converter uses `${record.date}` at
the template on the converter around the `${record.date}` usage), so update the
model to reflect runtime shape by changing the date type to string (or
alternatively parse the CSV value into a Date before constructing
ScalableCapitalRecord); locate the ScalableCapitalRecord class and change the
date field type to string to avoid the misleading Date annotation and runtime
mismatch.
| ### Scalable Capital | ||
|
|
||
| to come |
There was a problem hiding this comment.
Placeholder documentation — export instructions are missing.
The "Scalable Capital" section just says "to come". The PR checklist also shows the README update isn't completed. Please fill in the export instructions before merging, consistent with other broker sections.
🤖 Prompt for AI Agents
In `@README.md` around lines 166 - 168, The "Scalable Capital" README section
currently contains only a placeholder; replace the "to come" under the "###
Scalable Capital" heading with full export instructions consistent with other
broker sections (copy the structure and content pattern used by other broker
entries: required credentials, API endpoints, step-by-step export commands or UI
steps, expected file formats, and any environment variables), ensure you
reference the exact heading "### Scalable Capital" so the content sits in the
correct place, and update the PR checklist entry for README completion if
present.
| | Relai | `run start relai` | | ||
| | Revolut | `run start revolut` | | ||
| | Saxo | `run start saxo` | | ||
| | Scalable Capital | `run start scalablecapital` or (`sc`) | |
There was a problem hiding this comment.
Inconsistent alias format.
Other entries use (or \xx`)but this line usesor (`sc`)`. Align with the existing pattern for consistency.
-| Scalable Capital | `run start scalablecapital` or (`sc`) |
+| Scalable Capital | `run start scalablecapital` (or `sc`) |📝 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.
| | Scalable Capital | `run start scalablecapital` or (`sc`) | | |
| | Scalable Capital | `run start scalablecapital` (or `sc`) | |
🤖 Prompt for AI Agents
In `@README.md` at line 316, Update the table row for "Scalable Capital" to match
the existing alias format by changing the alias fragment from "or (`sc`)" to
"(or `sc`)"; locate the row containing the command text `run start
scalablecapital` and the alias `sc` and make the parentheses placement
consistent with other entries (i.e., use "(or `sc`)").
|
|
||
| it("should process sample CSV file", (done) => { | ||
|
|
||
| // Arange |
There was a problem hiding this comment.
Typo: "Arange" → "Arrange".
- // Arange
+ // Arrange📝 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.
| // Arange | |
| // Arrange |
🤖 Prompt for AI Agents
In `@src/converters/scalableCapitalConverter.test.ts` at line 27, Typo in the test
comment: change the misspelled "Arange" to "Arrange" in the
scalableCapitalConverter.test.ts test file (locate the comment inside the
relevant test/describe block where "Arange" appears) so the test comments read
"Arrange" for consistency and clarity.
| it("should log error and invoke errorCallback when an error occurs in processFileContents", (done) => { | ||
|
|
||
| // Arrange | ||
| const tempFileContent = "date;time;status;reference;description;assetType;type;isin;shares;price;amount;fee;tax;currency\n"; | ||
| const sut = new ScalableCapitalConverter(new SecurityService(new YahooFinanceServiceMock())); | ||
|
|
||
| const consoleSpy = jest.spyOn(console, "log"); | ||
|
|
||
| // Act | ||
| sut.processFileContents(tempFileContent, () => { | ||
| throw new Error("Should not succeed!"); | ||
| }, (err: Error) => { | ||
|
|
||
| // Assert | ||
| expect(consoleSpy).toHaveBeenCalledWith("[e] An error occurred while processing the file contents. Stack trace:"); | ||
| expect(consoleSpy).toHaveBeenCalledWith(err.stack); | ||
| expect(err).toBeTruthy(); | ||
|
|
||
| done(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "scalableCapitalConverter.ts" -not -path "*/node_modules/*" -type fRepository: dickwolff/Export-To-Ghostfolio
Length of output: 118
🏁 Script executed:
find . -name "scalableCapitalConverter.test.ts" -not -path "*/node_modules/*" -type fRepository: dickwolff/Export-To-Ghostfolio
Length of output: 123
🏁 Script executed:
cat -n ./src/converters/scalableCapitalConverter.tsRepository: dickwolff/Export-To-Ghostfolio
Length of output: 7048
🏁 Script executed:
cat -n ./src/converters/scalableCapitalConverter.test.ts | sed -n '148,168p'Repository: dickwolff/Export-To-Ghostfolio
Length of output: 1019
🏁 Script executed:
cat -n ./src/converters/scalableCapitalConverter.test.ts | sed -n '61,78p'Repository: dickwolff/Export-To-Ghostfolio
Length of output: 814
Test likely doesn't exercise the intended code path.
This test provides header-only content, which triggers the early-return error path in the converter (lines 66-74 of scalableCapitalConverter.ts). That path calls errorCallback directly via return errorCallback(new Error(...)) and does not throw an exception, so the outer catch block (converter lines 143-148) is never executed.
Therefore, the assertions on lines 162–163 (expect(consoleSpy).toHaveBeenCalledWith("[e] An error occurred while processing the file contents. Stack trace:") and the stack trace log) should not pass — the spy would never observe those log messages.
To test the catch-block path, you need input that passes parsing (non-empty records) but causes an exception during processing (e.g., by stubbing an internal method to throw).
🤖 Prompt for AI Agents
In `@src/converters/scalableCapitalConverter.test.ts` around lines 148 - 168, The
test currently uses header-only input and hits the early-return errorCallback
path instead of the converter's catch block; change the test to supply content
with at least one data row (so processFileContents continues past the header)
and then force an exception during processing by stubbing a method invoked
inside processFileContents (for example, spyOn the ScalableCapitalConverter
instance's internal processing helper like processRow or the specific private
method used to convert a row and make it throw). Call sut.processFileContents
with the same success and error callbacks and assert the consoleSpy sees the
"[e] An error occurred..." message and the stack trace in the error callback to
exercise the catch block (reference ScalableCapitalConverter.processFileContents
and the converter's internal row-processing helper).
| if (context.column === "type") { | ||
| const action = columnValue.toLocaleLowerCase(); | ||
|
|
||
| if (action.includes("savings plan")) { | ||
| return "buy"; | ||
| } | ||
| else if (action.includes("distribution")) { | ||
| return "dividend"; | ||
| } | ||
| else if (action.includes("taxes")) { | ||
| return "fee"; | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find GhostfolioOrderType enum definition
rg "enum GhostfolioOrderType" -A 10Repository: dickwolff/Export-To-Ghostfolio
Length of output: 453
🏁 Script executed:
# Check the actual file to understand type mapping and usage
cat -n src/converters/scalableCapitalConverter.ts | sed -n '20,130p'Repository: dickwolff/Export-To-Ghostfolio
Length of output: 5198
🏁 Script executed:
# Search for imports to verify dayjs plugin usage
head -n 15 src/converters/scalableCapitalConverter.tsRepository: dickwolff/Export-To-Ghostfolio
Length of output: 721
🏁 Script executed:
# Check how type is being used and what values CSV might contain
rg "type.*buy|type.*sell|type.*Buy|type.*Sell" -i src/converters/Repository: dickwolff/Export-To-Ghostfolio
Length of output: 6287
🏁 Script executed:
# Look for test data or documentation about Scalable Capital CSV format
rg "scalable|capital" -i src/converters/scalableCapitalConverter.test.ts -A 20 -B 5 | head -100Repository: dickwolff/Export-To-Ghostfolio
Length of output: 3342
🏁 Script executed:
# Check isIgnoredRecord to understand type filtering
cat -n src/converters/scalableCapitalConverter.ts | sed -n '150,165p'Repository: dickwolff/Export-To-Ghostfolio
Length of output: 366
🏁 Script executed:
# Search for any test data or sample CSVs
find . -name "*scalable*" -o -name "*capital*" 2>/dev/null | grep -E "\.(csv|test|spec)" | head -20Repository: dickwolff/Export-To-Ghostfolio
Length of output: 160
🏁 Script executed:
# Check the sample CSV to see actual type values
cat samples/scalablecapital-export.csvRepository: dickwolff/Export-To-Ghostfolio
Length of output: 4194
🏁 Script executed:
# Also check the complete type mapping logic and fee calculation
cat -n src/converters/scalableCapitalConverter.ts | sed -n '119,130p'Repository: dickwolff/Export-To-Ghostfolio
Length of output: 793
🏁 Script executed:
# Look for PR description or issue references
git log --oneline --all | grep -i "scalable\|fee\|tax" | head -10Repository: dickwolff/Export-To-Ghostfolio
Length of output: 525
🏁 Script executed:
# Check if there's any documentation about fee handling
rg "fee.*tax|tax.*fee" src/converters/scalableCapitalConverter.ts -B 3 -A 3Repository: dickwolff/Export-To-Ghostfolio
Length of output: 56
🏁 Script executed:
# Check how other converters handle fee/tax combination
rg "fee:.*tax|fee\s*\+.*tax" src/converters/ -iRepository: dickwolff/Export-To-Ghostfolio
Length of output: 177
🏁 Script executed:
# Verify if the dayjs format string actually works without customParseFormat
node -e "const dayjs = require('dayjs'); console.log(dayjs('2025-09-25 02:00:00', 'YYYY-MM-DD HH:mm:ss').format());"Repository: dickwolff/Export-To-Ghostfolio
Length of output: 906
🏁 Script executed:
# Check the test expectations for activities to see if fee calculation is tested
rg "expect.*fee" src/converters/scalableCapitalConverter.test.ts -B 2 -A 2Repository: dickwolff/Export-To-Ghostfolio
Length of output: 56
🏁 Script executed:
# Look at the git commit about combining fees
git show 3a5a4e9 --statRepository: dickwolff/Export-To-Ghostfolio
Length of output: 396
🏁 Script executed:
# Check how Distribution type records are expected to be processed
cat -n src/converters/scalableCapitalConverter.ts | sed -n '119,131p'Repository: dickwolff/Export-To-Ghostfolio
Length of output: 867
🏁 Script executed:
# Search for any reference to combining tax with fee in this converter
rg "tax" src/converters/scalableCapitalConverter.tsRepository: dickwolff/Export-To-Ghostfolio
Length of output: 178
🏁 Script executed:
# Look for similar patterns in other converters - how do they handle fee/tax?
rg "record\.fee.*record\.tax|record\.tax.*record\.fee" src/converters/ -B 2 -A 2Repository: dickwolff/Export-To-Ghostfolio
Length of output: 390
🏁 Script executed:
# Check if ScalableCapitalRecord has a definition to understand data types
rg "class ScalableCapitalRecord|interface ScalableCapitalRecord" -A 15Repository: dickwolff/Export-To-Ghostfolio
Length of output: 908
🏁 Script executed:
# Look at Distribution records specifically - do they have tax values?
cat samples/scalablecapital-export.csv | grep "Distribution"Repository: dickwolff/Export-To-Ghostfolio
Length of output: 636
🏁 Script executed:
# Check the test file more thoroughly for what activities are expected
cat src/converters/scalableCapitalConverter.test.ts | tail -100Repository: dickwolff/Export-To-Ghostfolio
Length of output: 4158
🏁 Script executed:
# Look at the parqetConverter to confirm the fee+tax pattern
cat -n src/converters/parqetConverter.ts | sed -n '110,135p'Repository: dickwolff/Export-To-Ghostfolio
Length of output: 1259
Combine tax and fee values for accurate transaction costs.
Line 125 should sum the tax and fee fields, similar to other converters in the codebase. The Scalable Capital CSV provides both tax and fee columns (e.g., Distribution records have separate fee and tax amounts), but currently only record.fee is used. Update to:
fee: Math.abs(record.fee) + Math.abs(record.tax),
Also note: the dayjs import (line 1) does not include the customParseFormat plugin, so the format string argument on line 119 is silently ignored. The parsing still works for this date format, but consider importing the plugin if format-specific parsing becomes necessary.
🤖 Prompt for AI Agents
In `@src/converters/scalableCapitalConverter.ts` around lines 32 - 44, Update the
fee mapping to combine tax and fee by replacing the single use of record.fee
with the sum Math.abs(record.fee) + Math.abs(record.tax) so transaction costs
include both fields (look for the code that builds the output object using
record.fee/record.tax). Also import and register dayjs's customParseFormat
plugin (add import of customParseFormat and call
dayjs.extend(customParseFormat)) so the explicit date format argument used with
dayjs(...) is honored.
| // Parse numbers to floats (from string). | ||
| if (context.column === "shares" || | ||
| context.column === "price" || | ||
| context.column === "amount" || | ||
| context.column === "fee" || | ||
| context.column === "tax") { | ||
| if (columnValue === "") { | ||
| return 0; | ||
| } | ||
|
|
||
| return Math.abs(Number.parseFloat(columnValue.replace(",", "."))); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how other converters handle comma decimal separators
rg -n 'replace.*\(' src/converters/ --type=ts -A 2 -B 2 | head -100Repository: dickwolff/Export-To-Ghostfolio
Length of output: 6847
🏁 Script executed:
#!/bin/bash
# Look at the specific file mentioned in the review
cat -n src/converters/scalableCapitalConverter.ts | head -70Repository: dickwolff/Export-To-Ghostfolio
Length of output: 3120
🏁 Script executed:
#!/bin/bash
# Search for test files related to scalableCapital
find . -type f -name "*scalable*" -o -name "*test*" | grep -i scalableRepository: dickwolff/Export-To-Ghostfolio
Length of output: 243
🏁 Script executed:
#!/bin/bash
# Check for any sample data or documentation mentioning format
rg -l "Scalable Capital" --type=ts --type=md --type=json | head -10Repository: dickwolff/Export-To-Ghostfolio
Length of output: 100
🏁 Script executed:
#!/bin/bash
# Check sample data format
cat ./samples/scalablecapital-export.csvRepository: dickwolff/Export-To-Ghostfolio
Length of output: 4194
🏁 Script executed:
#!/bin/bash
# Check test file
cat -n ./src/converters/scalableCapitalConverter.test.ts | head -150Repository: dickwolff/Export-To-Ghostfolio
Length of output: 6602
Fix decimal separator handling for amounts with thousands separators.
The sample CSV contains -5.743,51 (a withdrawal), which uses periods as thousands separators and commas as decimal separators. The current code replace(",", ".") produces -5.743.51, and parseFloat returns -5.743, silently truncating the amount.
Replace with:
columnValue.replace(/\./g, "").replace(",", ".")This removes all periods first (thousands separators), then converts the comma (decimal separator) to a period, correctly parsing -5743.51.
🤖 Prompt for AI Agents
In `@src/converters/scalableCapitalConverter.ts` around lines 46 - 57, The
number-parsing branch in scalableCapitalConverter (the if checking
context.column === "shares" || "price" || "amount" || "fee" || "tax") mishandles
numbers with period thousands separators and comma decimals; update the
transformation before Number.parseFloat to first strip all periods (thousands
separators) and then replace the comma with a dot (e.g., use a global remove of
"." then replace "," with ".") so "-5.743,51" becomes "-5743.51" before parsing;
keep the existing empty-string check and Math.abs(Number.parseFloat(...))
behavior unchanged.
| const date = dayjs(`${record.date} ${record.time}`, "YYYY-MM-DD HH:mm:ss"); | ||
|
|
||
| // Add record to export. | ||
| result.activities.push({ | ||
| accountId: process.env.GHOSTFOLIO_ACCOUNT_ID, | ||
| comment: `${record.reference} - ${record.description}`, | ||
| fee: record.fee, | ||
| quantity: record.shares, | ||
| type: GhostfolioOrderType[record.type], | ||
| unitPrice: record.price, | ||
| currency: record.currency, | ||
| dataSource: "YAHOO", | ||
| date: date.format("YYYY-MM-DDTHH:mm:ssZ"), | ||
| symbol: security.symbol, | ||
| tags: getTags() | ||
| }); |
There was a problem hiding this comment.
Tax is not included in the activity fee — deviates from PR objectives.
Per the issue requirements, the fee field should be abs(tax) + abs(fee). Currently only record.fee is used (line 125), and record.tax is parsed but never referenced in the activity construction. This means taxes are silently dropped from the output.
Suggested fix
result.activities.push({
accountId: process.env.GHOSTFOLIO_ACCOUNT_ID,
comment: `${record.reference} - ${record.description}`,
- fee: record.fee,
+ fee: record.fee + record.tax,
quantity: record.shares,🤖 Prompt for AI Agents
In `@src/converters/scalableCapitalConverter.ts` around lines 119 - 134, The
activity construction in scalableCapitalConverter.ts currently sets fee to
record.fee and omits tax; update the fee field in the result.activities.push
object to include both tax and fee as absolute values (e.g., fee:
Math.abs(record.tax || 0) + Math.abs(record.fee || 0)) so taxes are not
dropped—make this change inside the block that builds the activity (the object
pushed into result.activities) where accountId, comment, fee, quantity, etc. are
set.
| continue; | ||
| } | ||
|
|
||
| const date = dayjs(`${record.date} ${record.time}`, "YYYY-MM-DD HH:mm:ss"); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if other converters in the project import customParseFormat
rg -n "customParseFormat" --type=tsRepository: dickwolff/Export-To-Ghostfolio
Length of output: 2252
🏁 Script executed:
# Examine the scalableCapitalConverter.ts file to see current dayjs usage
cat -n src/converters/scalableCapitalConverter.ts | head -150Repository: dickwolff/Export-To-Ghostfolio
Length of output: 6768
🏁 Script executed:
# Check how dayjs is imported and used in the codebase
rg -n "import dayjs" --type=ts -A 3Repository: dickwolff/Export-To-Ghostfolio
Length of output: 9819
Import and extend the customParseFormat plugin for dayjs date parsing
On line 119, the code uses dayjs() with a format string argument, which requires the customParseFormat plugin. Without it, the format string is silently ignored. This is missing from the file, while nearly all other converters in the codebase correctly import and extend this plugin.
Suggested fix
import dayjs from "dayjs";
+import customParseFormat from "dayjs/plugin/customParseFormat";
+dayjs.extend(customParseFormat);📝 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.
| const date = dayjs(`${record.date} ${record.time}`, "YYYY-MM-DD HH:mm:ss"); | |
| import dayjs from "dayjs"; | |
| import customParseFormat from "dayjs/plugin/customParseFormat"; | |
| dayjs.extend(customParseFormat); |
🤖 Prompt for AI Agents
In `@src/converters/scalableCapitalConverter.ts` at line 119, The call to dayjs
with a format string in the expression that assigns const date =
dayjs(`${record.date} ${record.time}`, "YYYY-MM-DD HH:mm:ss") requires the
customParseFormat plugin to be loaded; add an import for customParseFormat from
'dayjs/plugin/customParseFormat' and call dayjs.extend(customParseFormat) at
module top (alongside other dayjs imports/extensions) so the provided format
string is respected by the parser.
| export class ScalableCapitalRecord { | ||
| date: Date; | ||
| time: string; | ||
| reference: string; | ||
| description: string; | ||
| assetType: string; | ||
| type: string; | ||
| isin: string; | ||
| shares: number; | ||
| price: number; | ||
| amount: number; | ||
| fee: number; | ||
| tax: number; | ||
| currency: string; | ||
| } No newline at end of file |
There was a problem hiding this comment.
date field is typed as Date but will be a string at runtime.
The CSV parser's cast function has no special handling for the date column, so it remains a raw string (e.g. "2025-09-25"). The Date type annotation is misleading. In the converter (line 119), it's used as ${record.date} in a string template, which works only because it's actually a string at runtime.
Suggested fix
export class ScalableCapitalRecord {
- date: Date;
+ date: string;
time: string;📝 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.
| export class ScalableCapitalRecord { | |
| date: Date; | |
| time: string; | |
| reference: string; | |
| description: string; | |
| assetType: string; | |
| type: string; | |
| isin: string; | |
| shares: number; | |
| price: number; | |
| amount: number; | |
| fee: number; | |
| tax: number; | |
| currency: string; | |
| } | |
| export class ScalableCapitalRecord { | |
| date: string; | |
| time: string; | |
| reference: string; | |
| description: string; | |
| assetType: string; | |
| type: string; | |
| isin: string; | |
| shares: number; | |
| price: number; | |
| amount: number; | |
| fee: number; | |
| tax: number; | |
| currency: string; | |
| } |
🤖 Prompt for AI Agents
In `@src/models/scalableCapitalRecord.ts` around lines 1 - 15, The
ScalableCapitalRecord.date property is declared as Date but CSV parsing leaves
it as a string (and converter uses `${record.date}` at the template on the
converter around the `${record.date}` usage), so update the model to reflect
runtime shape by changing the date type to string (or alternatively parse the
CSV value into a Date before constructing ScalableCapitalRecord); locate the
ScalableCapitalRecord class and change the date field type to string to avoid
the misleading Date annotation and runtime mismatch.
1 participant
Added
Fixes
Checklist
Related issue (if applicable)
Fixes #272