Repository navigation
Conversation
…ector matcher, once-only surfacing (#1057)
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 49 minutes. View limit details
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 |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ac4ed2ebee
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const parsed = parseChargeCandidatesExport(JSON.stringify(gmailDoc)); | ||
| candidates.push(...(parsed ?? []).filter((c) => c.source === "gmail")); |
There was a problem hiding this comment.
When GMAIL_ALERTS_PATH contains the documented {at, merchant, amountUsd, method?} rows, every alert is silently dropped: parseChargeCandidatesExport requires a source field equal to gmail or plaid, but the documented extraction format supplies neither. A valid Gmail-only receipt therefore produces no charge evidence or unmatched alert despite the producer reporting success. Add the Gmail source while normalizing these rows before candidate validation.
Useful? React with 👍 / 👎.
| await writeFile(outPath, JSON.stringify(feed, null, 2)); | ||
| if (pendingSurfacedKeys) { | ||
| await writeFile(chargesSurfacedStatePath, JSON.stringify({ surfacedKeys: pendingSurfacedKeys }, null, 2)).catch( |
There was a problem hiding this comment.
If the laptop is unreachable during a collection, these keys are permanently marked as surfaced before the feed reaches its consumer. In scripts/push-fleet-feed.sh, the collector runs before the Python transfer section; if that transfer fails, the next timer run recollects the feed, excludes the already-recorded charges, and overwrites the only feed containing their alerts. Consequently, those unmatched charges never appear on the laptop. Retain pending alerts across collections or acknowledge their keys only after successful delivery.
Useful? React with 👍 / 👎.
| export function unmatchedChargeKey(u: UnmatchedCharge): string { | ||
| return `${u.normalized}|${u.amountUsd.toFixed(2)}|${u.at}`; |
There was a problem hiding this comment.
When a Gmail alert is collected before its corresponding Plaid posting, the same purchase surfaces twice if their dates differ. For example, a Gmail Cursor charge dated October 26 is recorded under that exact date; when the matching Plaid charge dated October 28 arrives, matchCharges correctly merges the evidence and prefers the Plaid date, but this key changes and the planner treats it as fresh. Reconcile persisted identities using the matcher's cross-source date window or another stable purchase identity so adding source-of-record evidence does not notify again.
Useful? React with 👍 / 👎.
| await writeFile(tmp, JSON.stringify({ generatedAt: new Date().toISOString(), candidates }, null, 2)); | ||
| await rename(tmp, outPath); |
There was a problem hiding this comment.
When both input files are missing, unreadable, or invalid JSON, the producer still exits successfully and overwrites the last-known-good export with candidates: []. The collector accepts that empty array and replaces its prose-derived charges with zero matched rows, so a source-read failure erases existing evidence and disables the otherwise-working prose fallback. Track whether any source was successfully read and leave the existing export untouched when neither was available, distinguishing that failure from a successful read containing no transactions.
Useful? React with 👍 / 👎.
|
Closing as a duplicate of PR #2149 (same queue task 2990, same #1057 wiring scope). #2149 was opened first (2026-10-29T06:30Z, before this PR's shift claimed the task), its CI is green, and its producer is more complete. Per protocol §8, a salvage diff was done first and posted on the canonical PR: #2149 (comment) — nothing unique to lift, one flagged item (recurring-charge surfacing key) for the canonical holder to lift or explicitly decline before merge. This branch stays readable as the salvage source until #2149 merges. |
Implements #1057 (queue task 2990), resuming from the lane-D scoping checkpoint: the pure matcher core (
src/charges-match.ts) was already on main; this PR adds the remaining wiring.What changed
scripts/gather-charges.ts(new) — the Muse-side producer: normalizes a saved Plaidtransactions-getresponse + Gmail alert extractions into theChargeCandidate[]export at~/atlas/charges-export/latest.json(file-export convention, gather-quota family 2). Charge evidence only — no balances/debt read or written; pending Plaid rows skipped; the Citi-not-linked gap is untouched.src/charges-collect.ts(new) — collector-side, stateful half: export parsing (fail-soft, malformed rows dropped individually), service refs from the hub's SaaS list, and the persisted "already surfaced" planning for unmatched charges.scripts/collect-fleet-feed.ts— when the export is present and parseable, the hub charges section comes frommatchChargesinstead of the prose extractor; fresh unmatched SaaS charges go into the feed's newchargesUnmatchedsection and their keys persist tocharges-surfaced-state.jsonafter the feed is written, so each surfaces exactly once. No export → prose path unchanged.src/feed.ts/src/model.ts/src/sources/live.ts—chargesUnmatchedfeed field;AttentionFlaggains kindunmatched-charge; the live usage view renders each freshly surfaced charge as an attention flag.Verification
bun test test/charges-match.test.ts test/charges-collect.test.ts— 16/16 pass, including a collector fixture replay: one row per charge, Plaid source-of-record on cross-source duplicates, unmatched surfacing exactly once across replays, a new posting next period surfaces again.actions.test.tsattention badges) fails identically on clean main; pre-existing, unrelated.bunx tsc --noEmitclean.Remaining for full acceptance: a live producer run (agent with the Plaid/Gmail connectors saves the raw reads, runs
gather-charges.ts, next collector run publishes) — the queue task parks on this PR's CI and tracks it.Need help on this PR? Tag
@codesmith-botwith what you need.