Repository navigation
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Description Check |  |
Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | |
The title clearly and concisely describes the main changes: ledger accounting diagnostics and balance telemetry. |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
✨ Finishing Touches: 💡 1
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
🛠️ Fix failing CI checks: 💡
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 help to get the list of available commands.
GAlexIHU
force-pushed
the
codex/ledger-error-telemetry
branch
from
October 19, 2026 13:17
3f553b1 to
dcbd79d
Compare
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
This was referenced Oct 19, 2026
GAlexIHU
added this pull request to stack #5295
October 19, 2026 13:36
GAlexIHU
force-pushed
the
codex/ledger-error-telemetry
branch
from
October 19, 2026 13:39
dcbd79d to
301b485
Compare
turip
reviewed
Oct 20, 2026
| func (s *service) recognitionGroup(ctx context.Context, namespace, id string, groups map[string]ledger.TransactionGroup) (ledger.TransactionGroup, error) { | ||
| if id == "" { | ||
| return nil, fmt.Errorf("recognition lineage is missing its allocation or backing transaction group") | ||
| return nil, models.NewError("ledger_recognition_failed", "recognition lineage is missing its allocation or backing transaction group", models.WithAttribute("reason", "ledger_recognition_lineage_reference_missing")) |
Member
There was a problem hiding this comment.
Please use a constant for the error codes.
| }, | ||
| }, nil | ||
| case models.Error: | ||
| if ordinaryErrorSeverity == "" { |
Member
There was a problem hiding this comment.
Please add a comment here, that any generic error not wrapped using WrapAsValidationIssue is considered a system error.
GAlexIHU
force-pushed
the
codex/ledger-error-telemetry
branch
from
October 21, 2026 13:44
bea52da to
299ba20
Compare
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Negative FBO source minima emit a counter and diagnostic warning on every observation, including repeated reads. Logs include the source, query boundary, and minimum balance; the counter measures observations, not distinct failures.
Ledger accounting guards gain stable codes and reasons, surfaced by the shared error logger through wrapped and joined errors. Diagnostic traversal is bounded.
Introduce
models.Errorwith the existing structured-error capabilities while keepingmodels.ValidationIssuea distinct type. Both share the implementation and preserve cause traversal and identity through context enrichment. Billing propagates structured system failures instead of storing them as validation issues, even when their cause is a validation issue.Validation on the rebased stack: model, logging, HTTP, billing classification, PostgreSQL-backed ledger, and invoice error-propagation tests plus scoped Go lint passed.
Stack: #5252 → #5270 → #5294 → #5304. Depends on #5294.
Summary by CodeRabbit
New Features
Improvements
Documentation
This PR appears safe to merge; no new actionable issues remain.Summary
Adds ledger error codes and reasons, includes them in shared error logs, and counts each observed negative FBO source balance.
Diagram
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Ledger guard fails] --> B[models.Error with code and reason] B --> C[Shared error logger] C --> D[Log codes and reasons] B --> E{Explicit validation conversion?} E -->|No| F[Return operation failure] E -->|Yes| G[Create billing validation issue]Reviews (5) · Last reviewed commit: "refactor(ledger): name diagnostic error ..." · Reviewed by Greptile