Conversation
📝 WalkthroughWalkthroughAdds a new Typer-based CLI for sanpy with commands for configuration, discovery, data retrieval, and diagnostics; introduces cross-platform CLI config storage and output formatters; adds unit and integration tests; updates packaging to expose a Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant CLI as "CLI (san/cli.py)"
participant Config as "Config (san/cli_config.py)"
participant API as "Santiment API (san.*)"
participant Formatter as "Formatters (san/cli_formatters.py)"
participant Output as "Output (stdout/stderr)"
User->>CLI: Run command (e.g., `san get ...`)
CLI->>Config: _init_api_key() -> load env/flag/stored key
Config-->>CLI: api_key (or error)
CLI->>API: Call API function (with api_key, retries, timeout)
API-->>CLI: Response (DataFrame / list / dict) or raise SanError
CLI->>Formatter: format_* (data, fmt)
Formatter-->>CLI: formatted text
CLI->>Output: output(text)
Output-->>User: display result (table/json/csv)
alt Error
API-->>CLI: SanError / Exception
CLI->>Output: output(error_text, err=True)
Output-->>User: display error (stderr) and exit 1
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@san/tests/test_cli_integration.py`:
- Around line 130-131: The test currently uses a weak check `assert "datetime"
in data[0] or "value" in data[0]`; update the assertion to require both fields
by changing it to `assert "datetime" in data[0] and "value" in data[0]` (i.e.,
validate that both keys exist on the first element of `data`), so the structure
check for `data[0]` verifies both expected fields are present.
In `@san/tests/test_cli.py`:
- Around line 24-29: The fixture mock_api_key is unused; either remove the
unused pytest fixture definition named mock_api_key or attach it to tests that
require it by adding mock_api_key as a test parameter; if keeping it, ensure the
patch target "san.ApiConfig" is correct and referenced by tests (e.g., include
mock_api_key in test function signatures that rely on san.ApiConfig.api_key)
otherwise delete the entire mock_api_key fixture block to eliminate dead code.
🧹 Nitpick comments (8)
san/cli_config.py (2)
24-32: Potential type inconsistency withPathconversion.When
APPDATAorXDG_CONFIG_HOMEis not set,Path.home()returns aPathobject, but when the env var exists,os.environ.get()returns a string. WhilePath(base)on line 31 handles this, it would be cleaner to ensure consistent types.♻️ Suggested improvement
def get_config_dir() -> Path: """Get the platform-appropriate config directory.""" if os.name == "nt": # Windows - base = os.environ.get("APPDATA", Path.home()) + base = Path(os.environ.get("APPDATA", str(Path.home()))) else: # Linux/Mac - base = os.environ.get("XDG_CONFIG_HOME", Path.home() / ".config") + xdg_config = os.environ.get("XDG_CONFIG_HOME") + base = Path(xdg_config) if xdg_config else Path.home() / ".config" - config_dir = Path(base) / "sanpy" + config_dir = base / "sanpy" return config_dir
56-65: Silent error handling may hide config file corruption.When
_load_configencounters aJSONDecodeError, it silently returns an empty dict. This could mask a corrupted config file from the user. Consider logging a warning or informing the user that their config was reset.♻️ Suggested improvement
def _load_config() -> dict: - """Load config from file, return empty dict if doesn't exist.""" + """Load config from file, return empty dict if doesn't exist or is corrupted.""" config_path = get_config_path() if config_path.exists(): try: with open(config_path, "r") as f: return json.load(f) - except (json.JSONDecodeError, IOError): + except json.JSONDecodeError: + # Config file is corrupted, return empty and let next save overwrite + import sys + print(f"Warning: Config file '{config_path}' is corrupted, using defaults.", file=sys.stderr) + return {} + except IOError: return {} return {}san/tests/test_cli_integration.py (1)
321-338: Consider adding assertion for error message content.The test verifies the exit code and that "error" appears in stdout, but doesn't validate that the error is specifically about the invalid slug. This could help catch regressions in error messaging.
san/cli_formatters.py (2)
49-54: CSV and table formats produce identical output for lists.The
csvandtablebranches produce the same output ("\n".join(items)). For true CSV format, consider adding a header or quoting values. If this is intentional, a comment would clarify the design choice.
100-104: Inconsistent CSV escaping - consider using csv module.
format_dictuses thecsvmodule for proper escaping, butformat_api_callsuses string formatting. If a date string contains commas or special characters, this could produce malformed CSV.♻️ Suggested fix
elif fmt == "csv": - lines = ["date,api_calls"] - for date, count in calls: - lines.append(f"{date},{count}") - return "\n".join(lines) + buffer = io.StringIO() + writer = csv.writer(buffer, quoting=csv.QUOTE_MINIMAL) + writer.writerow(["date", "api_calls"]) + for date, count in calls: + writer.writerow([date, count]) + return buffer.getvalue()san/cli.py (2)
72-78: Consider security implications of displaying config path.Echoing the full config path after saving the API key is helpful for transparency, but ensure users understand the key is stored in plaintext.
♻️ Optional enhancement for security awareness
def config_set_key( api_key: Annotated[str, typer.Argument(help="Your Santiment API key")], ) -> None: """Store API key in config file.""" set_api_key(api_key) - output(f"API key saved to {get_config_path()}") + output(f"API key saved to {get_config_path()}") + output("Note: The API key is stored in plaintext.")
164-188:--slugis a required option - consider making it a positional argument.The
slugparameter is marked as required (no default value), which is correct. However, for better UX, consider whetherslugshould be a positional argument likemetric, enablingsan get price_usd bitcoininstead ofsan get price_usd --slug bitcoin.setup.py (1)
19-19: Update the typer version constraint.The
typer>=0.9.0constraint is significantly outdated. The latest stable version is0.21.1(released Jan 6, 2026). Consider updating the constraint totyper>=0.21.0or higher to benefit from bug fixes, performance improvements, and new features from recent releases.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
san/tests/test_cli.py (1)
24-29: 🛠️ Refactor suggestion | 🟠 MajorFixture is unused and should be removed.
The
mock_api_keyfixture is defined but never referenced by any test function. This is dead code that should be removed.♻️ Remove unused fixture
-@pytest.fixture -def mock_api_key(): - """Set up a mock API key for tests.""" - with patch("san.ApiConfig") as mock_config: - mock_config.api_key = "test_api_key_12345" - yield mock_config -🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@san/tests/test_cli.py` around lines 24 - 29, Remove the unused pytest fixture by deleting the mock_api_key function definition (the pytest.fixture block named mock_api_key) from the test file; also remove any now-unused imports (e.g., patch or other mocking helpers) that existed solely to support mock_api_key so there are no leftover unused imports or dead code.
🧹 Nitpick comments (3)
san/cli_config.py (2)
114-119: Config file persists even when empty after clearing.After clearing the API key, if no other config keys exist, the file will contain just
{}. This is fine, but consider whether you want to delete the file entirely when it becomes empty.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@san/cli_config.py` around lines 114 - 119, clear_api_key currently deletes "api_key" and writes an empty dict back via _save_config, leaving a {} file; change clear_api_key to call _load_config, delete "api_key", then if the resulting config is empty (len(config) == 0) remove the config file instead of calling _save_config: either call an existing helper to delete the config file or add a small helper (e.g., _remove_config_file) and invoke it from clear_api_key; otherwise call _save_config as before. Ensure you reference the existing helpers _load_config and _save_config and the function name clear_api_key when making the change.
56-65: Silent failure on corrupted config may mask issues.When
json.JSONDecodeErroris caught, the function silently returns an empty dict. While this prevents crashes, the user won't know their config file is corrupted. Consider logging a warning or informing the user.♻️ Optional: Add warning for corrupted config
def _load_config() -> dict: """Load config from file, return empty dict if doesn't exist.""" config_path = get_config_path() if config_path.exists(): try: with open(config_path, "r") as f: return json.load(f) - except (json.JSONDecodeError, IOError): + except json.JSONDecodeError: + import warnings + warnings.warn(f"Config file '{config_path}' is corrupted, using defaults") + return {} + except IOError: return {} return {}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@san/cli_config.py` around lines 56 - 65, The _load_config function currently swallows json.JSONDecodeError and returns {} silently; update it to catch JSONDecodeError separately and emit a warning that includes the config file path (from get_config_path()/config_path) and the error details before returning an empty dict so users know their config is corrupted; use the module logger (e.g. logging.getLogger(__name__)) or a clear print call and keep IOError handled as before.san/cli_formatters.py (1)
27-31: DataFrame mutation may affect caller's data.The
df.reset_index()result is assigned back todf, but the originaldfpassed in may still be mutated if it's a view. Whilereset_index()returns a new DataFrame by default, this reassignment shadows the parameter, which is fine. However, for absolute safety and clarity, consider making an explicit copy if the DataFrame might be reused.if fmt == "json": # Reset index to include datetime in output if df.index.name == "datetime" or isinstance(df.index, pd.DatetimeIndex): - df = df.reset_index() + df = df.reset_index() # Creates a new DataFrame, original is unchanged return df.to_json(orient="records", date_format="iso", indent=2)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@san/cli_formatters.py` around lines 27 - 31, In the fmt == "json" block avoid mutating the caller's DataFrame by operating on an explicit copy: create a local copy of df (e.g., df_copy = df.copy()) before calling reset_index() and to_json(), then use that copy for any reset_index()/to_json() calls so the original df passed in is never altered; update references to df in this block (reset_index and to_json) to use the new local copy.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@san/tests/test_cli.py`:
- Around line 24-29: Remove the unused pytest fixture by deleting the
mock_api_key function definition (the pytest.fixture block named mock_api_key)
from the test file; also remove any now-unused imports (e.g., patch or other
mocking helpers) that existed solely to support mock_api_key so there are no
leftover unused imports or dead code.
---
Nitpick comments:
In `@san/cli_config.py`:
- Around line 114-119: clear_api_key currently deletes "api_key" and writes an
empty dict back via _save_config, leaving a {} file; change clear_api_key to
call _load_config, delete "api_key", then if the resulting config is empty
(len(config) == 0) remove the config file instead of calling _save_config:
either call an existing helper to delete the config file or add a small helper
(e.g., _remove_config_file) and invoke it from clear_api_key; otherwise call
_save_config as before. Ensure you reference the existing helpers _load_config
and _save_config and the function name clear_api_key when making the change.
- Around line 56-65: The _load_config function currently swallows
json.JSONDecodeError and returns {} silently; update it to catch JSONDecodeError
separately and emit a warning that includes the config file path (from
get_config_path()/config_path) and the error details before returning an empty
dict so users know their config is corrupted; use the module logger (e.g.
logging.getLogger(__name__)) or a clear print call and keep IOError handled as
before.
In `@san/cli_formatters.py`:
- Around line 27-31: In the fmt == "json" block avoid mutating the caller's
DataFrame by operating on an explicit copy: create a local copy of df (e.g.,
df_copy = df.copy()) before calling reset_index() and to_json(), then use that
copy for any reset_index()/to_json() calls so the original df passed in is never
altered; update references to df in this block (reset_index and to_json) to use
the new local copy.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1ca20c81-8cba-43b6-86b6-90856b8c036b
📒 Files selected for processing (5)
san/cli.pysan/cli_config.pysan/cli_formatters.pysan/tests/test_cli.pysan/tests/test_cli_integration.py
🚧 Files skipped from review as they are similar to previous changes (1)
- san/tests/test_cli_integration.py
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@san/cli.py`:
- Around line 230-233: After parsing slugs into slug_list (from the slugs
variable) add a guard that validates slug_list is not empty before building
kwargs and calling san.get_many; if empty, surface a clear, immediate error
(e.g., raise a user-facing exception or exit with an error message) so the CLI
fails fast with an actionable message instead of allowing an empty list to
propagate into san.get_many; update the code around
slug_list/kwargs/san.get_many to perform this check and return/exit on failure.
- Around line 85-90: The CLI currently accepts the API key as a positional
argument in config_set_key which can expose secrets; change the command to
obtain the key securely (e.g., convert the api_key parameter from a positional
typer.Argument to a typed option/flag or remove the argument and prompt
interactively) and use Typer's prompt with hide_input=True or read from
STDIN/ENV before calling set_api_key; update config_set_key to call
set_api_key(api_key) and preserve the confirmation output using
get_config_path() after securely acquiring the value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4e83213e-feef-477e-9b99-13c9a4cd47cc
📒 Files selected for processing (5)
san/cli.pysan/cli_config.pysan/cli_formatters.pysan/tests/test_cli.pysan/tests/test_cli_integration.py
🚧 Files skipped from review as they are similar to previous changes (2)
- san/cli_config.py
- san/cli_formatters.py
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
san/cli.py (2)
229-233:⚠️ Potential issue | 🟡 MinorReject an empty parsed
--slugslist.Inputs like
--slugs ","currently become[]and flow intosan.get_many(...), so the user gets a downstream error instead of an actionable CLI validation message.Proposed fix
def get_many( @@ ) -> None: """Fetch timeseries data for a metric across multiple assets.""" _init_api_key(api_key) + slug_list = [s.strip() for s in slugs.split(",") if s.strip()] + if not slug_list: + raise typer.BadParameter("Provide at least one valid slug in --slugs.") try: - slug_list = [s.strip() for s in slugs.split(",") if s.strip()] kwargs = dict( slugs=slug_list, from_date=from_date, to_date=to_date,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@san/cli.py` around lines 229 - 233, After parsing slugs into slug_list, validate that slug_list is non-empty and if it is empty (e.g., user passed "--slugs ','" or only commas) raise a CLI-friendly error instead of letting an empty list flow into san.get_many; update the parsing block that creates slug_list (variable slug_list from input slugs) to detect an empty result and raise a click.BadParameter (or call sys.exit with a clear message) indicating the --slugs value is invalid so the user gets an actionable validation message.
84-90:⚠️ Potential issue | 🟠 MajorDon't take the API key as a positional argument.
This leaks the secret via shell history and process inspection. Use a hidden prompt or another non-argv input path instead; the README/tests will need the same interface change.
Proposed hardening
`@config_app.command`("set-key") def config_set_key( - api_key: Annotated[str, typer.Argument(help="Your Santiment API key")], + api_key: Annotated[ + Optional[str], + typer.Option( + "--api-key", + prompt=True, + hide_input=True, + help="Your Santiment API key", + ), + ] = None, ) -> None: """Store API key in config file.""" + if not api_key: + raise typer.BadParameter("API key is required.") set_api_key(api_key) output(f"API key saved to {get_config_path()}")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@san/cli.py` around lines 84 - 90, The command currently takes the API key as a positional argument (config_set_key) which leaks secrets; change the api_key parameter from an Argument to an Option that prompts hidden input (e.g., Annotated[str, typer.Option(..., prompt=True, hide_input=True)]) or alternatively read from getpass when no tty is available, then call set_api_key(api_key) and output the config path as before; update any README/tests that expect a positional arg to use the new interactive/hidden input or provide the option flag.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Around line 69-72: The fenced code block containing the commands "san --help"
and "san --version" lacks a language tag, which trips markdownlint and prevents
shell highlighting; update that block by changing the opening backticks to
include a language identifier (e.g., replace ``` with ```bash) so the block
reads ```bash followed by the two commands and closing ``` to enable shell
syntax highlighting.
- Line 111: The README currently claims "All commands support `--format`
(`table`, `json`, `csv`) and `--api-key` options" which is inaccurate; update
that sentence to scope the flag support to only the commands that `san/cli.py`
actually registers: the discovery, data, and diagnostic subcommands. Replace the
line with wording that explicitly states that `--format` and `--api-key` are
supported by the discovery, data, and diagnostic commands (and not by config
subcommands), referencing `san/cli.py` and the discovery/data/diagnostic command
names to make the scope clear.
In `@san/tests/test_cli.py`:
- Around line 105-115: The test currently asserts a generic "test" substring
which can collide with tmp path names; update test_config_show to assert the
masked token specifically by storing the key (e.g., key = "test_key_abcd1234"),
using runner.invoke(app, ["config", "set-key", key]) and then after
runner.invoke(app, ["config", "show"]) assert that the output contains the key's
visible prefix (e.g., key[:4] or the exact expected visible prefix) and that the
full key string is NOT present in result.stdout (or that the masked output
contains a masking character like '*' after the prefix), ensuring you reference
test_config_show, the same key variable, and the two runner.invoke calls when
making the change.
---
Duplicate comments:
In `@san/cli.py`:
- Around line 229-233: After parsing slugs into slug_list, validate that
slug_list is non-empty and if it is empty (e.g., user passed "--slugs ','" or
only commas) raise a CLI-friendly error instead of letting an empty list flow
into san.get_many; update the parsing block that creates slug_list (variable
slug_list from input slugs) to detect an empty result and raise a
click.BadParameter (or call sys.exit with a clear message) indicating the
--slugs value is invalid so the user gets an actionable validation message.
- Around line 84-90: The command currently takes the API key as a positional
argument (config_set_key) which leaks secrets; change the api_key parameter from
an Argument to an Option that prompts hidden input (e.g., Annotated[str,
typer.Option(..., prompt=True, hide_input=True)]) or alternatively read from
getpass when no tty is available, then call set_api_key(api_key) and output the
config path as before; update any README/tests that expect a positional arg to
use the new interactive/hidden input or provide the option flag.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b521df6c-39fc-4d09-b8c0-a8abad8b5788
📒 Files selected for processing (8)
README.mdexamples/extras/event_study.ipynbsan/cli.pysan/cli_config.pysan/cli_formatters.pysan/tests/test_cli.pysan/tests/test_cli_integration.pysetup.py
✅ Files skipped from review due to trivial changes (1)
- san/cli_config.py
🚧 Files skipped from review as they are similar to previous changes (3)
- setup.py
- san/tests/test_cli_integration.py
- san/cli_formatters.py
👮 Files not reviewed due to content moderation or server errors (1)
- examples/extras/event_study.ipynb
| ``` | ||
| san --help | ||
| san --version | ||
| ``` |
There was a problem hiding this comment.
Add a language to this fenced block.
This trips markdownlint and loses shell highlighting.
Proposed fix
-```
+```bash
san --help
san --version</details>
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.22.0)</summary>
[warning] 69-69: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against the current code and only fix it if needed.
In @README.md around lines 69 - 72, The fenced code block containing the
commands "san --help" and "san --version" lacks a language tag, which trips
markdownlint and prevents shell highlighting; update that block by changing the
opening backticks to include a language identifier (e.g., replace withbash) so the block reads bash followed by the two commands and closing
to enable shell syntax highlighting.
</details>
<!-- fingerprinting:phantom:medusa:grasshopper -->
<!-- This is an auto-generated comment by CodeRabbit -->
| san complexity price_usd # Query complexity check | ||
| ``` | ||
|
|
||
| All commands support `--format` (`table`, `json`, `csv`) and `--api-key` options. |
There was a problem hiding this comment.
Scope the flag-support claim more narrowly.
san/cli.py only adds --format/--api-key to the discovery/data/diagnostic commands. config subcommands don't accept those flags, so this sentence currently overstates the interface.
Proposed wording
-All commands support `--format` (`table`, `json`, `csv`) and `--api-key` options.
+The discovery, data, and diagnostics commands support `--format` (`table`, `json`, `csv`).
+API-backed commands also accept `--api-key`.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| All commands support `--format` (`table`, `json`, `csv`) and `--api-key` options. | |
| The discovery, data, and diagnostics commands support `--format` (`table`, `json`, `csv`). | |
| API-backed commands also accept `--api-key`. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` at line 111, The README currently claims "All commands support
`--format` (`table`, `json`, `csv`) and `--api-key` options" which is
inaccurate; update that sentence to scope the flag support to only the commands
that `san/cli.py` actually registers: the discovery, data, and diagnostic
subcommands. Replace the line with wording that explicitly states that
`--format` and `--api-key` are supported by the discovery, data, and diagnostic
commands (and not by config subcommands), referencing `san/cli.py` and the
discovery/data/diagnostic command names to make the scope clear.
| def test_config_show(tmp_path, monkeypatch): | ||
| """Test showing config.""" | ||
| monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_path)) | ||
|
|
||
| # First set a key | ||
| runner.invoke(app, ["config", "set-key", "test_key_abcd1234"]) | ||
|
|
||
| # Then show config | ||
| result = runner.invoke(app, ["config", "show"]) | ||
| assert result.exit_code == 0 | ||
| assert "test" in result.stdout # Masked key shows first chars |
There was a problem hiding this comment.
Assert the masked token, not a generic test substring.
This can pass even if masking regresses, because config show also prints a path under tmp_path, and pytest temp directories typically include test_....
Proposed fix
def test_config_show(tmp_path, monkeypatch):
"""Test showing config."""
monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_path))
# First set a key
- runner.invoke(app, ["config", "set-key", "test_key_abcd1234"])
+ set_result = runner.invoke(app, ["config", "set-key", "test_key_abcd1234"])
+ assert set_result.exit_code == 0
# Then show config
result = runner.invoke(app, ["config", "show"])
assert result.exit_code == 0
- assert "test" in result.stdout # Masked key shows first chars
+ assert "Stored API key: test...1234" in result.stdout
+ assert "test_key_abcd1234" not in result.stdout🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@san/tests/test_cli.py` around lines 105 - 115, The test currently asserts a
generic "test" substring which can collide with tmp path names; update
test_config_show to assert the masked token specifically by storing the key
(e.g., key = "test_key_abcd1234"), using runner.invoke(app, ["config",
"set-key", key]) and then after runner.invoke(app, ["config", "show"]) assert
that the output contains the key's visible prefix (e.g., key[:4] or the exact
expected visible prefix) and that the full key string is NOT present in
result.stdout (or that the masked output contains a masking character like '*'
after the prefix), ensuring you reference test_config_show, the same key
variable, and the two runner.invoke calls when making the change.
Summary by CodeRabbit