Skip to content
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ markers = [
"transactions: transaction-related tests",
"budgets: budget-related tests",
"insights: insight analysis tests",
"rules: rule management tests",
"unit: marks tests as unit tests",
]

Expand Down
91 changes: 91 additions & 0 deletions src/lampyrid/clients/firefly.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@
InsightGroup,
InsightTotal,
InsightTransfer,
RuleArray,
RuleSingle,
RuleUpdate,
TransactionArray,
TransactionSingle,
TransactionStore,
Expand Down Expand Up @@ -470,3 +473,91 @@ async def get_transfer_by_asset_account(
self._handle_api_error(r)
r.raise_for_status()
return InsightTransfer.model_validate(r.json())

# =========================================================================
# Rule Management Methods
# =========================================================================

async def get_rules(self, page: int = 1) -> RuleArray:
"""Get all rules with pagination."""
r = await self._client.get('/api/v1/rules', params={'page': page})
self._handle_api_error(r)
r.raise_for_status()
return RuleArray.model_validate(r.json())

async def get_rule(self, rule_id: str) -> RuleSingle:
"""Get a single rule by ID."""
r = await self._client.get(f'/api/v1/rules/{rule_id}')
self._handle_api_error(r)
r.raise_for_status()
return RuleSingle.model_validate(r.json())

async def update_rule(self, rule_id: str, rule_update: RuleUpdate) -> RuleSingle:
"""Update an existing rule."""
payload = self._serialize_model(rule_update, exclude_unset=True)
r = await self._client.put(f'/api/v1/rules/{rule_id}', json=payload)
self._handle_api_error(r, payload)
r.raise_for_status()
return RuleSingle.model_validate(r.json())

async def test_rule(
self,
rule_id: str,
start_date: date,
end_date: date,
account_ids: Optional[list[str]] = None,
) -> TransactionArray:
"""Test a rule in preview mode (shows matches without changes).

Args:
rule_id: ID of the rule to test
start_date: Start date for matching transactions
end_date: End date for matching transactions
account_ids: Optional list of account IDs to filter

Returns:
TransactionArray with matching transactions

"""
params: Dict[str, Any] = {
'start': start_date.strftime('%Y-%m-%d'),
'end': end_date.strftime('%Y-%m-%d'),
}
if account_ids:
params['accounts[]'] = account_ids

r = await self._client.get(f'/api/v1/rules/{rule_id}/test', params=params)
self._handle_api_error(r)
r.raise_for_status()
return TransactionArray.model_validate(r.json())
Comment on lines +503 to +532
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, let's find the trigger_rule method and examine its signature
rg -n "def trigger_rule" src/lampyrid/clients/firefly.py -A 10

Repository: RadCod3/LamPyrid

Length of output: 413


🏁 Script executed:

# Search for insight methods and their account_ids parameter types
rg -n "def.*insight" src/lampyrid/clients/firefly.py -A 10

Repository: RadCod3/LamPyrid

Length of output: 482


🏁 Script executed:

# Search for _build_insight_params method mentioned in the scratchpad
rg -n "_build_insight_params" src/lampyrid/clients/firefly.py -B 2 -A 15

Repository: RadCod3/LamPyrid

Length of output: 6951


🏁 Script executed:

# Get overall structure of the file to understand parameter patterns
wc -l src/lampyrid/clients/firefly.py

Repository: RadCod3/LamPyrid

Length of output: 95


🏁 Script executed:

# Let's check the context around how params['accounts[]'] is used
# and if there's any type conversion happening
rg -n "accounts\[\]" src/lampyrid/clients/firefly.py -B 2 -A 2

Repository: RadCod3/LamPyrid

Length of output: 563


🏁 Script executed:

# Check the imports to understand what types are expected
head -30 src/lampyrid/clients/firefly.py

Repository: RadCod3/LamPyrid

Length of output: 657


🏁 Script executed:

# Let's examine the test_rule and trigger_rule methods more closely
sed -n '503,550p' src/lampyrid/clients/firefly.py

Repository: RadCod3/LamPyrid

Length of output: 1655


🏁 Script executed:

# Let's search for other uses of account_ids to see the pattern
rg -n "account_id" src/lampyrid/clients/firefly.py | head -20

Repository: RadCod3/LamPyrid

Length of output: 1334


🏁 Script executed:

# Check if there's any documentation or comments about account ID types
rg -n "account" src/lampyrid/clients/firefly.py | grep -i "id" | head -15

Repository: RadCod3/LamPyrid

Length of output: 972


🏁 Script executed:

# Let's check if there are any test files or examples showing how account_ids are used
find . -type f -name "*.py" | xargs grep -l "test_rule\|trigger_rule" | head -5

Repository: RadCod3/LamPyrid

Length of output: 232


🏁 Script executed:

# Let's also check if there's any documentation about the Firefly III API in the repo
find . -type f \( -name "*.md" -o -name "*.txt" -o -name "*.rst" \) | xargs grep -l "account" 2>/dev/null | head -5

Repository: RadCod3/LamPyrid

Length of output: 71


🏁 Script executed:

# Check if there's type validation or conversion happening in the _client.get or _client.post methods
rg -n "class.*Client" src/ -A 5 | head -30

Repository: RadCod3/LamPyrid

Length of output: 526


🏁 Script executed:

# Let's check the test_rules_service.py to see how these methods are called
cat tests/unit/test_rules_service.py | head -100

Repository: RadCod3/LamPyrid

Length of output: 2496


🏁 Script executed:

# Let's check the rules.py service to see how it uses test_rule and trigger_rule
rg -n "test_rule\|trigger_rule" src/lampyrid/services/rules.py -B 3 -A 5

Repository: RadCod3/LamPyrid

Length of output: 42


🏁 Script executed:

# Let's check the README for any API documentation references
cat README.md | grep -i -A 5 -B 5 "account\|rule"

Repository: RadCod3/LamPyrid

Length of output: 2511


🏁 Script executed:

# Let's check how the RuleService uses test_rule and trigger_rule
cat src/lampyrid/services/rules.py

Repository: RadCod3/LamPyrid

Length of output: 11440


🏁 Script executed:

# Let's look for any API documentation or comments about account IDs
rg -n "accounts\[\]|account.*id|rule.*id" src/lampyrid/clients/firefly.py | grep -i "str\|int" | head -20

Repository: RadCod3/LamPyrid

Length of output: 1052


🏁 Script executed:

# Let's check the TestRuleRequest and ExecuteRuleRequest to see what account_ids type they expect
rg -n "class.*Request" src/lampyrid/models/lampyrid_models.py -A 15 | grep -A 15 "TestRuleRequest\|ExecuteRuleRequest"

Repository: RadCod3/LamPyrid

Length of output: 1222


🏁 Script executed:

# Let's also verify if there are any type conversion issues when passing account_ids to the API
rg -n "Optional\[list" src/lampyrid/ -A 1

Repository: RadCod3/LamPyrid

Length of output: 2052


🏁 Script executed:

# Let's see the full definition of TestRuleRequest and ExecuteRuleRequest including account_ids field
sed -n '1209,1237p' src/lampyrid/models/lampyrid_models.py

Repository: RadCod3/LamPyrid

Length of output: 964


🏁 Script executed:

# And ExecuteRuleRequest
sed -n '1238,1270p' src/lampyrid/models/lampyrid_models.py

Repository: RadCod3/LamPyrid

Length of output: 1242


Standardize account_ids parameter types across rule and insight methods.

The test_rule and trigger_rule methods use account_ids: Optional[list[str]], while all insight methods use account_ids: Optional[list[int]]. Both pass these to the same query parameter key (accounts[]), so the types should be consistent. Additionally, the corresponding request models (TestRuleRequest and ExecuteRuleRequest) also specify List[str], creating an inconsistency throughout the interface. Either unify all account filtering to list[int] (matching insight methods) or clarify why rule endpoints require different types.

🤖 Prompt for AI Agents
In `@src/lampyrid/clients/firefly.py` around lines 503 - 532, The rule-related
APIs and their request models currently use account_ids: Optional[list[str]]
(e.g., test_rule and ExecuteRuleRequest/TestRuleRequest) while insight methods
use Optional[list[int]]; unify them by changing all rule method signatures,
query param handling, and request models to use Optional[list[int]] (or pick int
consistently) so the shared query key 'accounts[]' always receives integers;
update test_rule, trigger_rule, TestRuleRequest, ExecuteRuleRequest and any
other rule-insight callers to accept/serialize list[int] and ensure
params['accounts[]'] passes ints (or converts incoming ints to strings only at
the HTTP layer if necessary).


async def trigger_rule(
self,
rule_id: str,
start_date: date,
end_date: date,
account_ids: Optional[list[str]] = None,
) -> bool:
"""Execute a rule (applies changes to matching transactions).

Args:
rule_id: ID of the rule to execute
start_date: Start date for matching transactions
end_date: End date for matching transactions
account_ids: Optional list of account IDs to filter

Returns:
True if the rule execution was accepted (processing is async)

"""
params: Dict[str, Any] = {
'start': start_date.strftime('%Y-%m-%d'),
'end': end_date.strftime('%Y-%m-%d'),
}
if account_ids:
params['accounts[]'] = account_ids

r = await self._client.post(f'/api/v1/rules/{rule_id}/trigger', params=params)
self._handle_api_error(r)
r.raise_for_status()
return r.status_code == 204
Loading
Loading