Conversation
📝 WalkthroughWalkthroughAdds a salary management feature: new Changes
Sequence DiagramssequenceDiagram
actor User
participant CLI as "Account Salary CLI"
participant Config as "Config Module"
participant Pluggy as "Pluggy API"
participant Validators as "Validators"
participant UI as "Interactive UI"
User->>CLI: arcter account salary --sync
CLI->>Config: Load account.salary_filters
Config-->>CLI: Return filters
CLI->>Pluggy: List transactions (date range)
Pluggy-->>CLI: Transactions
CLI->>Validators: Filter by category & amount
Validators-->>CLI: Matched transactions
alt Exactly one match
CLI->>Config: Persist salary (silent)
Config-->>CLI: Ack
CLI-->>User: (no output)
else Multiple matches
CLI->>UI: Show options
UI->>User: Prompt selection
User-->>UI: Choice
UI-->>CLI: Selected transaction
CLI->>Config: Persist salary
Config-->>CLI: Ack
CLI-->>User: Confirmation
else No matches
CLI->>UI: Show debug tips / prompt manual set
UI-->>User: Tips
end
sequenceDiagram
actor User
participant CLI as "Transactions Listing"
participant Config as "Config Module"
participant Pluggy as "Pluggy API"
participant Salary as "Salary Sync Logic"
User->>CLI: arcter account transactions
CLI->>Config: Load account.salary_filters
Config-->>CLI: Filters present?
CLI->>Pluggy: List transactions
Pluggy-->>CLI: Transactions
alt Filters configured
CLI->>Salary: Find salary matches
Salary->>Salary: Apply category & amount filters
alt Exactly one match
Salary->>Config: Auto-sync salary (silent)
Config-->>Salary: Ack
else Not exactly one
Salary-->>CLI: No auto-sync
end
end
CLI-->>User: Show transactions
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
No actionable comments were generated in the recent review. 🎉 Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@AGENTS.md`:
- Line 132: Remove the unintended `develop` branch trigger: edit the CI workflow
(ci.yml) to delete `develop` from the push branches list (so `on.push.branches`
only contains `master`) and update the AGENTS.md documentation line that
currently says “pushes to master and develop” to state only “pushes to master
and all pull requests”; ensure both the workflow (ci.yml) and AGENTS.md are
consistent and that the workflow still triggers for pull requests.
In `@tests/test_account_cli.py`:
- Around line 681-953: The test file tests/test_account_cli.py currently
contains 79 integration tests but the repo guideline requires exactly 70; either
remove or consolidate 9 tests (e.g., merge similar cases among functions like
test_account_salary_set_updates_config,
test_account_salary_rejects_conflicting_options,
test_account_salary_requires_sync_or_set_option,
test_account_salary_sync_sets_salary_when_single_match_found,
test_account_salary_sync_prompts_selection_when_multiple_matches_found,
test_account_salary_sync_shows_debug_tips_and_fallback_selection_when_no_match)
to bring the total to 70, or update the project guideline/config that enforces
the 70-test limit to reflect the new baseline; locate the test-count enforcement
(CI script or test suite metadata) and adjust accordingly if you choose to
change the guideline.
In `@tests/test_config_cli.py`:
- Around line 231-306: The test suite now has 16 integration tests which exceeds
the 12-test guideline because the two new functions
test_set_salary_filters_writes_nested_account_table and
test_set_salary_filter_amount_rejects_invalid_expression add extra coverage; fix
by consolidating these into a single test (e.g., combine the "set" + "get"
assertions and the invalid-expression check into one unified test) or remove
redundant assertions so total integration tests return to 12; specifically,
merge the logic of test_set_salary_filters_writes_nested_account_table and
test_set_salary_filter_amount_rejects_invalid_expression into one function
(keeping key assertions that validate writing nested account.salary_filters,
reading them back, and rejecting invalid expressions) or drop non-essential
assertions within those functions to reduce the count.
|
|
||
| ## CI/CD | ||
| The GitHub Actions workflow (`.github/workflows/ci.yml`) triggers on pushes to `master` and all pull requests: | ||
| The GitHub Actions workflow (`.github/workflows/ci.yml`) triggers on pushes to `master` and `develop`, plus all pull requests: |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect CI workflow file to check the actual triggers
cat .github/workflows/ci.ymlRepository: geavenx/arcter
Length of output: 796
Remove develop branch from CI triggers to match intended design.
The documentation currently states CI runs on pushes to master and develop, which matches the current .github/workflows/ci.yml configuration. However, per the intended specification, CI should trigger only on pushes to master and all pull requests—the develop branch trigger should not be included. Update both the workflow file and this documentation line to remove develop.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~132-~132: The official name of this software platform is spelled with a capital “H”.
Context: ... ## CI/CD The GitHub Actions workflow (.github/workflows/ci.yml) triggers on pushes t...
(GITHUB)
🤖 Prompt for AI Agents
In `@AGENTS.md` at line 132, Remove the unintended `develop` branch trigger: edit
the CI workflow (ci.yml) to delete `develop` from the push branches list (so
`on.push.branches` only contains `master`) and update the AGENTS.md
documentation line that currently says “pushes to master and develop” to state
only “pushes to master and all pull requests”; ensure both the workflow (ci.yml)
and AGENTS.md are consistent and that the workflow still triggers for pull
requests.
Comment on lines
+681
to
+953
| def test_account_salary_set_updates_config(tmp_path: Path, env) -> None: | ||
| result = runner.invoke( | ||
| app, | ||
| ["account", "salary", "--set", "4322.75"], | ||
| env=env(), | ||
| ) | ||
| assert result.exit_code == 0 | ||
| assert "Set salary = 4322.75" in result.stdout | ||
|
|
||
| get_result = runner.invoke(app, ["config", "get", "salary"], env=env()) | ||
| assert get_result.exit_code == 0 | ||
| assert "salary = 4322.75 (source: file)" in get_result.stdout | ||
|
|
||
|
|
||
| def test_account_salary_rejects_conflicting_options(tmp_path: Path, env) -> None: | ||
| result = runner.invoke( | ||
| app, | ||
| ["account", "salary", "--sync", "--set", "4300"], | ||
| env=env(), | ||
| ) | ||
| output = f"{result.stdout}{result.stderr}" | ||
|
|
||
| assert result.exit_code == 1 | ||
| assert "Options --sync and --set are mutually exclusive." in output | ||
|
|
||
|
|
||
| def test_account_salary_requires_sync_or_set_option(tmp_path: Path, env) -> None: | ||
| result = runner.invoke( | ||
| app, | ||
| ["account", "salary"], | ||
| env=env(), | ||
| ) | ||
| output = f"{result.stdout}{result.stderr}" | ||
|
|
||
| assert result.exit_code == 1 | ||
| assert "Provide one of --sync or --set." in output | ||
|
|
||
|
|
||
| def test_account_salary_sync_sets_salary_when_single_match_found( | ||
| tmp_path: Path, env, monkeypatch | ||
| ) -> None: | ||
| observed: dict[str, object] = {} | ||
|
|
||
| def fake_load_config() -> Config: | ||
| return Config( | ||
| currency=ISO4217("BRL"), | ||
| salary=Decimal("200.00"), | ||
| savings_goal=Decimal("500.00"), | ||
| account={"salary_filters": {"category": "Transfer", "amount": [">=4300"]}}, | ||
| ) | ||
|
|
||
| def fake_current_month_date_range( | ||
| today: datetime.date | None = None, | ||
| ) -> tuple[str, str]: | ||
| return ("2026-02-01", "2026-02-12") | ||
|
|
||
| def fake_list_item_transactions_with_env( | ||
| item_id: str | None, | ||
| date_from: str | None = None, | ||
| date_to: str | None = None, | ||
| account_type_filter: str | None = None, | ||
| env: dict[str, str] | None = None, | ||
| ) -> list[TransactionRow]: | ||
| observed["item_id"] = item_id | ||
| observed["date_from"] = date_from | ||
| observed["date_to"] = date_to | ||
| return [ | ||
| TransactionRow( | ||
| date="2026-02-05", | ||
| description="Monthly salary", | ||
| amount=Decimal("4322.44"), | ||
| currency_code="BRL", | ||
| type="CREDIT", | ||
| status="POSTED", | ||
| category="Transfer", | ||
| account_name="Checking", | ||
| account_type="BANK", | ||
| ), | ||
| TransactionRow( | ||
| date="2026-02-06", | ||
| description="Groceries", | ||
| amount=Decimal("150.00"), | ||
| currency_code="BRL", | ||
| type="DEBIT", | ||
| status="POSTED", | ||
| category="Food", | ||
| account_name="Checking", | ||
| account_type="BANK", | ||
| ), | ||
| ] | ||
|
|
||
| monkeypatch.setattr("arcter.commands.account.load_config", fake_load_config) | ||
| monkeypatch.setattr( | ||
| "arcter.commands.account.validators.current_month_date_range", | ||
| fake_current_month_date_range, | ||
| ) | ||
| monkeypatch.setattr( | ||
| "arcter.commands.account.pluggy.list_item_transactions_with_env", | ||
| fake_list_item_transactions_with_env, | ||
| ) | ||
|
|
||
| result = runner.invoke(app, ["account", "salary", "--sync"], env=env()) | ||
|
|
||
| assert result.exit_code == 0 | ||
| assert observed["item_id"] is None | ||
| assert observed["date_from"] == "2026-02-01" | ||
| assert observed["date_to"] == "2026-02-12" | ||
| assert "Set salary = 4322.44" in result.stdout | ||
|
|
||
|
|
||
| def test_account_salary_sync_prompts_selection_when_multiple_matches_found( | ||
| tmp_path: Path, env, monkeypatch | ||
| ) -> None: | ||
| observed: dict[str, str] = {} | ||
|
|
||
| def fake_load_config() -> Config: | ||
| return Config( | ||
| currency=ISO4217("BRL"), | ||
| salary=Decimal("200.00"), | ||
| savings_goal=Decimal("500.00"), | ||
| account={"salary_filters": {"category": "Transfer", "amount": []}}, | ||
| ) | ||
|
|
||
| def fake_current_month_date_range( | ||
| today: datetime.date | None = None, | ||
| ) -> tuple[str, str]: | ||
| return ("2026-02-01", "2026-02-12") | ||
|
|
||
| def fake_list_item_transactions_with_env( | ||
| item_id: str | None, | ||
| date_from: str | None = None, | ||
| date_to: str | None = None, | ||
| account_type_filter: str | None = None, | ||
| env: dict[str, str] | None = None, | ||
| ) -> list[TransactionRow]: | ||
| return [ | ||
| TransactionRow( | ||
| date="2026-02-05", | ||
| description="Salary 1", | ||
| amount=Decimal("4300.00"), | ||
| currency_code="BRL", | ||
| type="CREDIT", | ||
| status="POSTED", | ||
| category="Transfer", | ||
| account_name="Checking", | ||
| account_type="BANK", | ||
| ), | ||
| TransactionRow( | ||
| date="2026-02-06", | ||
| description="Salary 2", | ||
| amount=Decimal("4380.12"), | ||
| currency_code="BRL", | ||
| type="CREDIT", | ||
| status="POSTED", | ||
| category="Transfer", | ||
| account_name="Savings", | ||
| account_type="BANK", | ||
| ), | ||
| ] | ||
|
|
||
| def fake_set_user_config(key: str, value: str) -> tuple[str, str]: | ||
| observed["key"] = key | ||
| observed["value"] = value | ||
| return key, value | ||
|
|
||
| monkeypatch.setattr("arcter.commands.account.load_config", fake_load_config) | ||
| monkeypatch.setattr( | ||
| "arcter.commands.account.validators.current_month_date_range", | ||
| fake_current_month_date_range, | ||
| ) | ||
| monkeypatch.setattr( | ||
| "arcter.commands.account.pluggy.list_item_transactions_with_env", | ||
| fake_list_item_transactions_with_env, | ||
| ) | ||
| monkeypatch.setattr("arcter.commands.account.set_user_config", fake_set_user_config) | ||
|
|
||
| result = runner.invoke( | ||
| app, | ||
| ["account", "salary", "--sync"], | ||
| env=env(), | ||
| input="2\n", | ||
| ) | ||
|
|
||
| assert result.exit_code == 0 | ||
| assert observed["key"] == "salary" | ||
| assert observed["value"] == "4380.12" | ||
| assert "Multiple transactions matched salary filters." in result.stdout | ||
| assert "Salary 1" in result.stdout | ||
| assert "Salary 2" in result.stdout | ||
| assert "Set salary = 4380.12" in result.stdout | ||
|
|
||
|
|
||
| def test_account_salary_sync_shows_debug_tips_and_fallback_selection_when_no_match( | ||
| tmp_path: Path, env, monkeypatch | ||
| ) -> None: | ||
| observed: dict[str, str] = {} | ||
|
|
||
| def fake_load_config() -> Config: | ||
| return Config( | ||
| currency=ISO4217("BRL"), | ||
| salary=Decimal("200.00"), | ||
| savings_goal=Decimal("500.00"), | ||
| account={"salary_filters": {"category": "Salary", "amount": [">=10000"]}}, | ||
| ) | ||
|
|
||
| def fake_current_month_date_range( | ||
| today: datetime.date | None = None, | ||
| ) -> tuple[str, str]: | ||
| return ("2026-02-01", "2026-02-12") | ||
|
|
||
| def fake_list_item_transactions_with_env( | ||
| item_id: str | None, | ||
| date_from: str | None = None, | ||
| date_to: str | None = None, | ||
| account_type_filter: str | None = None, | ||
| env: dict[str, str] | None = None, | ||
| ) -> list[TransactionRow]: | ||
| return [ | ||
| TransactionRow( | ||
| date="2026-02-05", | ||
| description="Salary picked manually", | ||
| amount=Decimal("4322.44"), | ||
| currency_code="BRL", | ||
| type="CREDIT", | ||
| status="POSTED", | ||
| category="Transfer", | ||
| account_name="Checking", | ||
| account_type="BANK", | ||
| ), | ||
| TransactionRow( | ||
| date="2026-02-06", | ||
| description="Groceries", | ||
| amount=Decimal("120.00"), | ||
| currency_code="BRL", | ||
| type="DEBIT", | ||
| status="POSTED", | ||
| category="Food", | ||
| account_name="Checking", | ||
| account_type="BANK", | ||
| ), | ||
| ] | ||
|
|
||
| def fake_set_user_config(key: str, value: str) -> tuple[str, str]: | ||
| observed["key"] = key | ||
| observed["value"] = value | ||
| return key, value | ||
|
|
||
| monkeypatch.setattr("arcter.commands.account.load_config", fake_load_config) | ||
| monkeypatch.setattr( | ||
| "arcter.commands.account.validators.current_month_date_range", | ||
| fake_current_month_date_range, | ||
| ) | ||
| monkeypatch.setattr( | ||
| "arcter.commands.account.pluggy.list_item_transactions_with_env", | ||
| fake_list_item_transactions_with_env, | ||
| ) | ||
| monkeypatch.setattr("arcter.commands.account.set_user_config", fake_set_user_config) | ||
|
|
||
| result = runner.invoke( | ||
| app, | ||
| ["account", "salary", "--sync"], | ||
| env=env(), | ||
| input="1\n", | ||
| ) | ||
|
|
||
| assert result.exit_code == 0 | ||
| assert observed["key"] == "salary" | ||
| assert observed["value"] == "4322.44" | ||
| assert "No transaction matched the configured salary filters." in result.stdout | ||
| assert "Tips:" in result.stdout | ||
| assert "Select a transaction from the current month to set salary:" in result.stdout | ||
| assert "Set salary = 4322.44" in result.stdout | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -c '^def test_' tests/test_account_cli.pyRepository: geavenx/arcter
Length of output: 60
Test count in tests/test_account_cli.py exceeds guideline.
The file currently contains 79 tests but the coding guideline specifies it should contain exactly 70 integration tests. Either consolidate/remove 9 tests or update the guideline requirement to reflect the new baseline.
🤖 Prompt for AI Agents
In `@tests/test_account_cli.py` around lines 681 - 953, The test file
tests/test_account_cli.py currently contains 79 integration tests but the repo
guideline requires exactly 70; either remove or consolidate 9 tests (e.g., merge
similar cases among functions like test_account_salary_set_updates_config,
test_account_salary_rejects_conflicting_options,
test_account_salary_requires_sync_or_set_option,
test_account_salary_sync_sets_salary_when_single_match_found,
test_account_salary_sync_prompts_selection_when_multiple_matches_found,
test_account_salary_sync_shows_debug_tips_and_fallback_selection_when_no_match)
to bring the total to 70, or update the project guideline/config that enforces
the 70-test limit to reflect the new baseline; locate the test-count enforcement
(CI script or test suite metadata) and adjust accordingly if you choose to
change the guideline.
Comment on lines
+231
to
+306
| def test_set_salary_filters_writes_nested_account_table(tmp_path: Path, env) -> None: | ||
| set_category_result = runner.invoke( | ||
| app, | ||
| ["config", "set", "account.salary_filters.category", "Transfer"], | ||
| env=env(), | ||
| ) | ||
| assert set_category_result.exit_code == 0 | ||
| assert ( | ||
| "Set account.salary_filters.category = Transfer" in set_category_result.stdout | ||
| ) | ||
|
|
||
| first_amount_result = runner.invoke( | ||
| app, | ||
| ["config", "set", "account.salary_filters.amount", ">=4300"], | ||
| env=env(), | ||
| ) | ||
| assert first_amount_result.exit_code == 0 | ||
| assert ( | ||
| 'Set account.salary_filters.amount = [">=4300"]' in first_amount_result.stdout | ||
| ) | ||
|
|
||
| second_amount_result = runner.invoke( | ||
| app, | ||
| ["config", "set", "account.salary_filters.amount", "<=4380"], | ||
| env=env(), | ||
| ) | ||
| assert second_amount_result.exit_code == 0 | ||
| assert ( | ||
| 'Set account.salary_filters.amount = [">=4300", "<=4380"]' | ||
| in second_amount_result.stdout | ||
| ) | ||
|
|
||
| config_file = tmp_path / "arcter" / "config.toml" | ||
| content = config_file.read_text(encoding="utf-8") | ||
| assert "[account.salary_filters]" in content | ||
| assert 'category = "Transfer"' in content | ||
| assert "amount = [" in content | ||
| assert '">=4300"' in content | ||
| assert '"<=4380"' in content | ||
|
|
||
| get_category_result = runner.invoke( | ||
| app, | ||
| ["config", "get", "account.salary_filters.category"], | ||
| env=env(), | ||
| ) | ||
| assert get_category_result.exit_code == 0 | ||
| assert ( | ||
| "account.salary_filters.category = Transfer (source: file)" | ||
| in get_category_result.stdout | ||
| ) | ||
|
|
||
| get_amount_result = runner.invoke( | ||
| app, | ||
| ["config", "get", "account.salary_filters.amount"], | ||
| env=env(), | ||
| ) | ||
| assert get_amount_result.exit_code == 0 | ||
| assert ( | ||
| 'account.salary_filters.amount = [">=4300", "<=4380"] (source: file)' | ||
| in get_amount_result.stdout | ||
| ) | ||
|
|
||
|
|
||
| def test_set_salary_filter_amount_rejects_invalid_expression( | ||
| tmp_path: Path, env | ||
| ) -> None: | ||
| result = runner.invoke( | ||
| app, | ||
| ["config", "set", "account.salary_filters.amount", "4300-ish"], | ||
| env=env(), | ||
| ) | ||
| output = f"{result.stdout}{result.stderr}" | ||
|
|
||
| assert result.exit_code == 1 | ||
| assert "Invalid salary amount filter expression." in output | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Count test functions in tests/test_config_cli.py
rg -n '^def test_' tests/test_config_cli.py | wc -lRepository: geavenx/arcter
Length of output: 60
Test count exceeds guideline: 16 tests instead of 12.
The file currently contains 16 test functions, exceeding the guideline of 12 integration tests. The new salary-filter tests added (lines 231–306) push the count above the requirement. Either reduce the test count to 12 by consolidating tests, or update the guideline to reflect the new intended count.
🤖 Prompt for AI Agents
In `@tests/test_config_cli.py` around lines 231 - 306, The test suite now has 16
integration tests which exceeds the 12-test guideline because the two new
functions test_set_salary_filters_writes_nested_account_table and
test_set_salary_filter_amount_rejects_invalid_expression add extra coverage; fix
by consolidating these into a single test (e.g., combine the "set" + "get"
assertions and the invalid-expression check into one unified test) or remove
redundant assertions so total integration tests return to 12; specifically,
merge the logic of test_set_salary_filters_writes_nested_account_table and
test_set_salary_filter_amount_rejects_invalid_expression into one function
(keeping key assertions that validate writing nested account.salary_filters,
reading them back, and rejecting invalid expressions) or drop non-essential
assertions within those functions to reduce the count.
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
1 participant
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.
Summary by CodeRabbit
New Features
Documentation
Tests