Sampler: pass template names newline-separated so spaced account template names survive

[michieldegezelle]

Account template identifiers are directory names and routinely contain spaces (account_templates/Investment- and depreciation details). The handles / account_templates inputs were documented and encoded as space-separated strings, then deliberately word-split into CLI args — so -at received Investment-, and, depreciation, details and the run died:

[error] Config file for account template "Investment-" not found

Hit live in lu_market: https://github.com/silverfin/lu_market/actions/runs/30612771053 (the PR under review changed exactly one file, inside that template).

Change

Both inputs are now one name per line, and the arg arrays are built with mapfile so each line becomes exactly one argv entry. This is already the convention in this repo's own run_tests.yml (TEMPLATE_BUCKETS, with a comment saying why) — the sampler was the only place that regressed to space-joining. firm_ids stays space-separated; it's numeric, so the SC2086 split there is still intentional.

Two supporting bits in the same area:

• The attempt log line is %q-quoted, so a name containing spaces is visibly one argument.
• The PR comment renders the names as individually code-quoted, comma-separated entries — a multi-line value inside one backtick pair renders as garbage.

⚠️ Input contract change for callers

A caller still passing a space-joined list now fails loudly (Config file for account template "..." not found) instead of silently sampling templates nobody asked for. I deliberately did not add a space-splitting fallback: for names containing spaces it cannot be made correct, and the loud failure is the honest outcome.

Input descriptions and the README input list are updated to say one per line. The only callers today are the lu_market / nl_market wrappers, which currently run inlined forks of this job (pinned to silverfin-cli's unmerged sampler-compact-diff-v2 branch); their matching fixes are silverfin/lu_market#784 and silverfin/nl_market#913. When those forks are reverted to uses: after silverfin-cli #265 merges, this fix means they don't regress — but the newline-emitting classify half lives in the market repos, so the two halves must land together.

Testing

The name-splitting logic was verified in bash against the real lu_market names (Investment- and depreciation details + Loan details → 3 argv entries, spaces intact); YAML validated. Not yet exercised in a live sampler run — see the market PRs for that.

🤖 Generated with Claude Code

[claude]

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.

_{Tip: disable this comment in your organization's Code Review settings.}

[coderabbitai]

Walkthrough

Changes

Sampler workflow

Layer / File(s)	Summary
Newline-delimited sampler inputs .github/workflows/run_sampler.yml, README.md	The workflow accepts each handle and account-template name as one newline-delimited argument. The CLI installation uses a pinned commit. Attempt logs quote argument boundaries.
Result download and artifact enrichment .github/workflows/run_sampler.yml, README.md	Failed workflows download results when a report URL exists. The workflow adds a best-effort diffs/ folder and uploads the artifact after a successful download.
Pull request result reporting .github/workflows/run_sampler.yml	Pull request comments prefer the seven-day artifact link, report enrichment failures, and use the presigned report URL only as a fallback.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

• silverfin/bso_github_actions#30: Introduced the workflow later refined by these input and reporting changes.
• silverfin/bso_github_actions#35: Overlaps with the pinned CLI, diffs enrichment, artifact, and comment reporting changes.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description explains the changes and testing, but it omits the required template sections for issue linkage, change type, and checklist.	Add the required Description, Fixes #, Type of change, and Checklist sections, and mark the applicable options.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly identifies the primary change: newline-separated template names that preserve spaces.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch sampler-report-link-and-diffs-folder

---

_{Comment @coderabbitai help to get the list of available commands.}

[michieldegezelle]

Superseded by #37 — same one-commit fix, but branched off current main so the PR carries only the newline-separated-template-names change. This branch was the already-merged sampler-report-link-and-diffs-folder branch reused, which made the commit list look like it re-contained the work merged in the July 29 PR.