feat: PR-6 add data processing for collation, export, and presets#6
Conversation
📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughAdds three new modules: Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
3 issues found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/export.ts">
<violation number="1" location="src/export.ts:9">
P1: CSV export does not neutralize spreadsheet formula prefixes for untrusted fields, allowing CSV injection when files are opened in spreadsheet tools.</violation>
</file>
<file name="src/presets.ts">
<violation number="1" location="src/presets.ts:38">
P1: Do not ignore `loadPresets` failures in `savePreset`; this can silently overwrite the presets file and cause data loss.</violation>
</file>
<file name="src/collate.ts">
<violation number="1" location="src/collate.ts:43">
P2: Use a null-prototype map (or `Map`) for dynamic-key counters; plain object dictionaries can miscount or behave unexpectedly for prototype-colliding keys.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Add result aggregation with stats by channel/author/embed type, file export in JSON and CSV formats with proper escaping, and preset persistence to local JSON files.
7c9824d to
917e7e0
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/export.ts`:
- Around line 5-10: The CSV escaping misses carriage returns; update the
escapeCsvField function to treat '\r' (and thus CRLF) like '\n' by including
"\r" in the conditional that triggers quoting and leaving the existing
double-quote escaping logic intact, so fields containing CR or CRLF are wrapped
in quotes and internal quotes are doubled.
In `@src/presets.ts`:
- Line 3: Replace the generic ExportError usage in preset-related code with a
new semantically appropriate PresetError: add a PresetError class (or type) and
export it, then update all references that currently import or throw ExportError
in preset functions (e.g., any load/save/delete preset handlers in presets.ts
and their callers) to import and throw PresetError instead so callers can
distinguish preset failures from export failures; ensure the presets.ts import
line is changed from ExportError to PresetError, update any error construction
messages to reflect "preset" operations, and update tests/type hints that
asserted ExportError to expect PresetError.
- Around line 20-21: The code unsafely casts JSON.parse(text) to Preset[]
without runtime validation; replace this with parsing followed by Zod
validation: import or define a PresetZodSchema (analogous to SearchParamsSchema
in src/discord/schemas.ts), run the schema.parse or safeParse on the parsed
object (the result of JSON.parse(text)), and handle failures by throwing or
returning a clear error/log instead of trusting the cast; update the code that
uses Preset[] to rely on the validated value from PresetZodSchema.parse so
downstream access to SearchParams fields is safe.
- Around line 37-38: The code silently swallows errors from loadPresets() by
converting failures to an empty array (presetsResult -> presets), which masks
real failures; change the logic in the loadPresets handling so that when
presetsResult.isOk() is false you propagate or throw the underlying error
instead of returning [] (do the same fix in deletePreset where a similar pattern
exists); locate the handling around the loadPresets call and the deletePreset
function and replace the fallback-to-empty-array behavior with explicit error
propagation (e.g., return or throw the presetsResult.error) so callers (like
savePreset) can detect and handle file/parse permission or corruption failures.
- Line 5: Move the PRESETS_FILE constant out of presets.ts into paths.ts
following the SETTINGS_FILE pattern (export const PRESETS_FILE = join(APP_DIR,
".discord-search-presets.json")), then update presets.ts to import {
PRESETS_FILE } from paths.ts and replace direct string usage in Bun.file(...)
and Bun.write(...) with the imported PRESETS_FILE; ensure you use the same
join(APP_DIR, ...) pattern and keep APP_DIR and join imports consistent with how
SETTINGS_FILE is defined.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e8dd0f4f-0618-4e68-8e80-fd2f1dd049c1
📒 Files selected for processing (3)
src/collate.tssrc/export.tssrc/presets.ts
…agation - Add \r to escapeCsvField quoting check in export.ts - Add PresetError tagged error class in errors.ts - Move PRESETS_FILE constant to paths.ts following SETTINGS_FILE pattern - Validate preset JSON with Zod schema instead of unsafe cast - Propagate loadPresets errors instead of silently falling back to [] Co-authored-by: Amp <amp@ampcode.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/presets.ts`:
- Around line 16-84: The loadPresets/savePreset/deletePreset functions currently
read/write PRESETS_FILE directly; update them to use the central config APIs
instead: call loadConfig() inside loadPresets to get config.presets (validate
with PresetsArraySchema if needed) and return that; in savePreset and
deletePreset, call loadConfig(), mutate the config.presets array (update or
remove the {name, params} entry), then call saveConfig(updatedConfig) to
persist; keep existing Result.tryPromise error wrapping and preserve validation
via PresetsArraySchema/SearchParamsSchema and throw/return PresetError on
failures.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9893abff-3465-4dac-a38d-bb025e06d96f
📒 Files selected for processing (4)
src/errors.tssrc/export.tssrc/paths.tssrc/presets.ts
Summary
Stacked PR Chain
This is PR 6 of 7 — merge in order.
main← PR1 ← PR2 ← PR3 ← PR4 ← PR5 ← PR6 ← PR7Test plan
bun run typecheckpassesbun run checkpasses lintingSummary by cubic
Add data collation, JSON/CSV exports, and preset storage to make Discord search results easier to analyze and reuse.
ExportErrorviabetter-result..discord-search-presets.json; validate preset JSON withzod; wrap and propagate errors withPresetError.Written for commit 36456de. Summary will update on new commits.