Repository navigation
fix: thread export main detection - #8
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe thread merger now receives a channel ID and uses it to identify the main export. The exporter passes this ID, tests cover missing starter messages, and ChangesChannel-aware thread merging
Docker context exclusions
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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.
Pull request overview
This PR makes thread-export merging more deterministic by requiring the exported channel ID when merging, and using that ID to identify the “main” channel export by filename rather than by inspecting message contents. It also adds a regression test for thread exports that omit starter messages and tightens Docker build context exclusions.
Changes:
- Update
mergeThreadsInDirectoryto requirechannelIdand select the main export by matching the filename trailing ID to that channel ID. - Update the exporter and tests to pass
channelIdintomergeThreadsInDirectory. - Add a test covering the case where a thread export omits its starter message; update
.dockerignoreto exclude.envand/data.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/services/thread-merger.ts |
Changes main-export detection to use the provided channelId and improves the related error message. |
src/services/exporter.ts |
Passes channelId into the updated merge function during channel export flow. |
tests/thread-merger.test.ts |
Updates existing tests for the new function signature and adds coverage for missing-starter thread exports. |
.dockerignore |
Excludes .env and /data from Docker build context to reduce leakage/unnecessary files. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This pull request updates the logic for merging channel and thread exports, making the identification of the main export file more robust and explicit by requiring the channel ID as a parameter. It also improves test coverage for edge cases and updates configuration files to avoid leaking sensitive or unnecessary data.
Main export identification and merging logic:
mergeThreadsInDirectoryinsrc/services/thread-merger.tsto require thechannelIdparameter and identify the main export file by matching the trailing ID in its filename to the provided channel ID, rather than by inspecting message contents. This makes the process more reliable and explicit. [1] [2]mergeThreadsInDirectoryinsrc/services/exporter.tsand tests to pass thechannelIdargument. [1] [2] [3] [4]Test improvements:
Configuration updates:
.dockerignoreto exclude.envand/datadirectories, preventing sensitive or unnecessary files from being included in Docker builds.Summary by CodeRabbit
Bug Fixes
Tests