Conversation
📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughAdds budget creation end-to-end: new CreateBudgetRequest model, BudgetService.create_budget(req) update, MCP tool create_budget, FireflyClient.delete_budget, model coercion for currency_id, and tests/fixtures to exercise creation and cleanup. Changes
Sequence Diagram(s)sequenceDiagram
participant MCP_Tool as "MCP Tool\ncreate_budget"
participant BudgetService as "BudgetService\n.create_budget(req)"
participant FireflyClient as "FireflyClient\n.create_budget / .delete_budget"
participant FireflyAPI as "Firefly III API\n(/api/v1/budgets)"
rect rgba(200,230,201,0.5)
MCP_Tool->>BudgetService: call create_budget(req)
end
rect rgba(187,222,251,0.5)
BudgetService->>FireflyClient: map req -> BudgetStore, call create_budget(store)
end
rect rgba(255,224,178,0.5)
FireflyClient->>FireflyAPI: POST /api/v1/budgets (body)
FireflyAPI-->>FireflyClient: 201 Created (budget JSON)
FireflyClient-->>BudgetService: Budget model
BudgetService-->>MCP_Tool: Budget result
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 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 |
coderabbitai
bot
changed the title
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #70 +/- ##
==========================================
- Coverage 97.62% 97.56% -0.07%
==========================================
Files 19 19
Lines 3080 3118 +38
==========================================
+ Hits 3007 3042 +35
- Misses 73 76 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
RadCod3
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/lampyrid/models/lampyrid_models.py`:
- Around line 701-740: Add a model-level validator on CreateBudgetRequest that
enforces the documented combinations: when auto_budget_type is set (i.e., not
None and not 'none') then auto_budget_amount and auto_budget_period must be
non-None (and optionally validate auto_budget_amount > 0 is already covered); if
those fields are missing raise a ValueError with a clear message. Implement this
using pydantic v2's `@model_validator`(mode='after') on the CreateBudgetRequest
class and reference the fields auto_budget_type, auto_budget_amount, and
auto_budget_period in the check.
🧹 Nitpick comments (2)
tests/fixtures/budgets.py (1)
71-90: Guard against incomplete auto-budget payloads in the factory.If
auto_budget_typeis set, the API expects an amount and period. The helper currently allows missing values, which can lead to invalid requests in tests. Consider adding a small validation guard.🔧 Suggested guard
def make_create_budget_request( name: str, auto_budget_type: Literal['none', 'reset', 'rollover'] | None = None, auto_budget_amount: float | None = None, auto_budget_period: Literal['daily', 'weekly', 'monthly', 'quarterly', 'half-year', 'yearly'] | None = None, auto_budget_currency_code: str | None = None, active: bool = True, notes: str | None = None, ) -> CreateBudgetRequest: """Create a CreateBudgetRequest for testing.""" + if auto_budget_type is not None and ( + auto_budget_amount is None or auto_budget_period is None + ): + raise ValueError( + "auto_budget_amount and auto_budget_period are required when auto_budget_type is set" + ) return CreateBudgetRequest( name=name, auto_budget_type=auto_budget_type, auto_budget_amount=auto_budget_amount, auto_budget_period=auto_budget_period, auto_budget_currency_code=auto_budget_currency_code, active=active, notes=notes, )tests/integration/test_budgets.py (1)
183-291: Consider using the new CreateBudgetRequest factory for validated payloads.These tests construct raw dicts; using
make_create_budget_request()would validate inputs and keep tests aligned with model changes. Example for one test (apply similarly to others):♻️ Example adjustment
+from tests.fixtures.budgets import make_create_budget_request ... async def test_create_budget_simple(mcp_client: Client, budget_cleanup: List[str]): """Test creating a basic budget with just a name.""" - result = await mcp_client.call_tool('create_budget', {'req': {'name': 'Test Created Budget'}}) + req = make_create_budget_request(name='Test Created Budget') + result = await mcp_client.call_tool('create_budget', {'req': req.model_dump()}) budget = Budget.model_validate(result.structured_content) budget_cleanup.append(budget.id)
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@README.md`:
- Around line 162-170: The Budget Management Markdown table needs proper spacing
and surrounding blank lines to satisfy MD058/MD060: add a blank line before the
table (after the "### Budget Management (6 tools)" heading if needed) and a
blank line after the table, and update the header and separator rows so each
cell and each pipe has spaces (e.g., ensure the header row uses " | " between
columns and the separator row uses " | --- | --- | " style with spaces around
pipes). Apply these changes to the "Budget Management" table that lists tools
like list_budgets, get_budget, get_budget_spending, get_budget_summary,
get_available_budget, and create_budget.
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.
This PR adds an MCP tool to create budgets. Also adds a temp fix for available budgets which seems to have broken again.