Merge pull request #9013 from freqtrade/feat/backtest_notes

Add backtest notes capability

freqtrade/data/btanalysis.py

@@ -12,7 +12,7 @@ import pandas as pd
 
 from freqtrade.constants import LAST_BT_RESULT_FN, IntOrInf
 from freqtrade.exceptions import OperationalException
-from freqtrade.misc import json_load
+from freqtrade.misc import file_dump_json, json_load
 from freqtrade.optimize.backtest_caching import get_backtest_metadata_filename
 from freqtrade.persistence import LocalTrade, Trade, init_db
 from freqtrade.types import BacktestHistoryEntryType, BacktestResultType
@@ -175,6 +175,21 @@ def _get_backtest_files(dirname: Path) -> List[Path]:
     return list(reversed(sorted(dirname.glob('backtest-result-*-[0-9][0-9].json'))))
 
 
+def get_backtest_result(filename: Path) -> List[BacktestHistoryEntryType]:
+    """
+    Get backtest result read from metadata file
+    """
+    return [
+        {
+            'filename': filename.stem,
+            'strategy': s,
+            'notes': v.get('notes', ''),
+            'run_id': v['run_id'],
+            'backtest_start_time': v['backtest_start_time'],
+        } for s, v in load_backtest_metadata(filename).items()
+    ]
+
+
 def get_backtest_resultlist(dirname: Path) -> List[BacktestHistoryEntryType]:
     """
     Get list of backtest results read from metadata files
@@ -184,6 +199,7 @@ def get_backtest_resultlist(dirname: Path) -> List[BacktestHistoryEntryType]:
             'filename': filename.stem,
             'strategy': s,
             'run_id': v['run_id'],
+            'notes': v.get('notes', ''),
             'backtest_start_time': v['backtest_start_time'],
         }
         for filename in _get_backtest_files(dirname)
@@ -203,6 +219,21 @@ def delete_backtest_result(file_abs: Path):
     file_abs_meta.unlink()
 
 
+def update_backtest_metadata(filename: Path, strategy: str, content: Dict[str, Any]):
+    """
+    Updates backtest metadata file with new content.
+    :raises: ValueError if metadata file does not exist, or strategy is not in this file.
+    """
+    metadata = load_backtest_metadata(filename)
+    if not metadata:
+        raise ValueError("File does not exist.")
+    if strategy not in metadata:
+        raise ValueError("Strategy not in metadata.")
+    metadata[strategy].update(content)
+    # Write data again.
+    file_dump_json(get_backtest_metadata_filename(filename), metadata)
+
+
 def find_existing_backtest_stats(dirname: Union[Path, str], run_ids: Dict[str, str],
                                  min_backtest_date: Optional[datetime] = None) -> Dict[str, Any]:
     """

freqtrade/optimize/optimize_reports/bt_storage.py

@@ -12,7 +12,7 @@ logger = logging.getLogger(__name__)
 
 
 def store_backtest_stats(
-        recordfilename: Path, stats: BacktestResultType, dtappendix: str) -> None:
+        recordfilename: Path, stats: BacktestResultType, dtappendix: str) -> Path:
     """
     Stores backtest results
     :param recordfilename: Path object, which can either be a filename or a directory.
@@ -41,6 +41,8 @@ def store_backtest_stats(
     latest_filename = Path.joinpath(filename.parent, LAST_BT_RESULT_FN)
     file_dump_json(latest_filename, {'latest_backtest': str(filename.name)})
 
+    return filename
+
 
 def _store_backtest_analysis_data(
         recordfilename: Path, data: Dict[str, Dict],

freqtrade/rpc/api_server/api_backtest.py

@@ -10,14 +10,15 @@ from fastapi.exceptions import HTTPException
 
 from freqtrade.configuration.config_validation import validate_config_consistency
 from freqtrade.constants import Config
-from freqtrade.data.btanalysis import (delete_backtest_result, get_backtest_resultlist,
+from freqtrade.data.btanalysis import (delete_backtest_result, get_backtest_result,
-                                       load_and_merge_backtest_result)
+                                       get_backtest_resultlist, load_and_merge_backtest_result,
+                                       update_backtest_metadata)
 from freqtrade.enums import BacktestState
 from freqtrade.exceptions import DependencyException, OperationalException
 from freqtrade.exchange.common import remove_exchange_credentials
 from freqtrade.misc import deep_merge_dicts, is_file_in_dir
-from freqtrade.rpc.api_server.api_schemas import (BacktestHistoryEntry, BacktestRequest,
+from freqtrade.rpc.api_server.api_schemas import (BacktestHistoryEntry, BacktestMetadataUpdate,
-                                                  BacktestResponse)
+                                                  BacktestRequest, BacktestResponse)
 from freqtrade.rpc.api_server.deps import get_config
 from freqtrade.rpc.api_server.webserver_bgwork import ApiBG
 from freqtrade.rpc.rpc import RPCException
@@ -74,10 +75,11 @@ def __run_backtest_bg(btconfig: Config):
         ApiBG.bt['bt'].load_prior_backtest()
 
         ApiBG.bt['bt'].abort = False
+        strategy_name = strat.get_strategy_name()
         if (ApiBG.bt['bt'].results and
-                strat.get_strategy_name() in ApiBG.bt['bt'].results['strategy']):
+                strategy_name in ApiBG.bt['bt'].results['strategy']):
             # When previous result hash matches - reuse that result and skip backtesting.
-            logger.info(f'Reusing result of previous backtest for {strat.get_strategy_name()}')
+            logger.info(f'Reusing result of previous backtest for {strategy_name}')
         else:
             min_date, max_date = ApiBG.bt['bt'].backtest_one_strategy(
                 strat, ApiBG.bt['data'], ApiBG.bt['timerange'])
@@ -87,10 +89,12 @@ def __run_backtest_bg(btconfig: Config):
                 min_date=min_date, max_date=max_date)
 
         if btconfig.get('export', 'none') == 'trades':
-            store_backtest_stats(
+            fn = store_backtest_stats(
                 btconfig['exportfilename'], ApiBG.bt['bt'].results,
                 datetime.now().strftime("%Y-%m-%d_%H-%M-%S")
                 )
+            ApiBG.bt['bt'].results['metadata'][strategy_name]['filename'] = str(fn.name)
+            ApiBG.bt['bt'].results['metadata'][strategy_name]['strategy'] = strategy_name
 
         logger.info("Backtest finished.")
 
@@ -281,3 +285,24 @@ def api_delete_backtest_history_entry(file: str, config=Depends(get_config)):
 
     delete_backtest_result(file_abs)
     return get_backtest_resultlist(config['user_data_dir'] / 'backtest_results')
+
+
+@router.patch('/backtest/history/{file}', response_model=List[BacktestHistoryEntry],
+              tags=['webserver', 'backtest'])
+def api_update_backtest_history_entry(file: str, body: BacktestMetadataUpdate,
+                                      config=Depends(get_config)):
+    # Get backtest result history, read from metadata files
+    bt_results_base: Path = config['user_data_dir'] / 'backtest_results'
+    file_abs = (bt_results_base / file).with_suffix('.json')
+    # Ensure file is in backtest_results directory
+    if not is_file_in_dir(file_abs, bt_results_base):
+        raise HTTPException(status_code=404, detail="File not found.")
+    content = {
+        'notes': body.notes
+    }
+    try:
+        update_backtest_metadata(file_abs, body.strategy, content)
+    except ValueError as e:
+        raise HTTPException(status_code=400, detail=str(e))
+
+    return get_backtest_result(file_abs)

freqtrade/rpc/api_server/api_schemas.py

@@ -526,6 +526,12 @@ class BacktestHistoryEntry(BaseModel):
     strategy: str
     run_id: str
     backtest_start_time: int
+    notes: Optional[str] = ''
+
+
+class BacktestMetadataUpdate(BaseModel):
+    strategy: str
+    notes: str = ''
 
 
 class SysInfo(BaseModel):

freqtrade/rpc/api_server/api_v1.py

@@ -50,7 +50,8 @@ logger = logging.getLogger(__name__)
 # 2.29: Add /exchanges endpoint
 # 2.30: new /pairlists endpoint
 # 2.31: new /backtest/history/ delete endpoint
-API_VERSION = 2.31
+# 2.32: new /backtest/history/ patch endpoint
+API_VERSION = 2.32
 
 # Public API, requires no auth.
 router_public = APIRouter()

freqtrade/types/backtest_result_type.py

@@ -25,3 +25,4 @@ def get_BacktestResultType_default() -> BacktestResultType:
 class BacktestHistoryEntryType(BacktestMetadataType):
     filename: str
     strategy: str
+    notes: str

tests/rpc/test_rpc_apiserver.py

@@ -10,6 +10,7 @@ from unittest.mock import ANY, MagicMock, PropertyMock
 
 import pandas as pd
 import pytest
+import rapidjson
 import uvicorn
 from fastapi import FastAPI, WebSocketDisconnect
 from fastapi.exceptions import HTTPException
@@ -80,6 +81,16 @@ def client_post(client: TestClient, url, data={}):
                                 })
 
 
+def client_patch(client: TestClient, url, data={}):
+
+    return client.patch(url,
+                        json=data,
+                        headers={'Authorization': _basic_auth_str(_TEST_USER, _TEST_PASS),
+                                 'Origin': 'http://example.com',
+                                 'content-type': 'application/json'
+                                 })
+
+
 def client_get(client: TestClient, url):
     # Add fake Origin to ensure CORS kicks in
     return client.get(url, headers={'Authorization': _basic_auth_str(_TEST_USER, _TEST_PASS),
@@ -1763,7 +1774,7 @@ def test_api_pairlists_evaluate(botclient, tmpdir, mocker):
     rc = client_get(client, f"{BASE_URI}/pairlists/evaluate/{job_id}")
     assert_response(rc)
     response = rc.json()
-    assert response['result']['whitelist'] == ['ETH/BTC', 'LTC/BTC', 'XRP/BTC', 'NEO/BTC',]
+    assert response['result']['whitelist'] == ['ETH/BTC', 'LTC/BTC', 'XRP/BTC', 'NEO/BTC']
     assert response['result']['length'] == 4
 
     # Restart with additional filter, reducing the list to 2
@@ -2010,6 +2021,7 @@ def test_api_backtest_history(botclient, mocker, testdatadir):
     assert len(result) == 3
     fn = result[0]['filename']
     assert fn == "backtest-result_multistrat"
+    assert result[0]['notes'] == ''
     strategy = result[0]['strategy']
     rc = client_get(client, f"{BASE_URI}/backtest/history/result?filename={fn}&strategy={strategy}")
     assert_response(rc)
@@ -2023,7 +2035,7 @@ def test_api_backtest_history(botclient, mocker, testdatadir):
     assert result2['backtest_result']['strategy'][strategy]
 
 
-def test_api_delete_backtest_history_entry(botclient, mocker, tmp_path: Path):
+def test_api_delete_backtest_history_entry(botclient, tmp_path: Path):
     ftbot, client = botclient
 
     # Create a temporary directory and file
@@ -2051,6 +2063,75 @@ def test_api_delete_backtest_history_entry(botclient, mocker, tmp_path: Path):
     assert not meta_path.exists()
 
 
+def test_api_patch_backtest_history_entry(botclient, tmp_path: Path):
+    ftbot, client = botclient
+
+    # Create a temporary directory and file
+    bt_results_base = tmp_path / "backtest_results"
+    bt_results_base.mkdir()
+    file_path = bt_results_base / "test.json"
+    file_path.touch()
+    meta_path = file_path.with_suffix('.meta.json')
+    with meta_path.open('w') as metafile:
+        rapidjson.dump({
+            CURRENT_TEST_STRATEGY: {
+                "run_id": "6e542efc8d5e62cef6e5be0ffbc29be81a6e751d",
+                "backtest_start_time": 1690176003}
+            }, metafile)
+
+    def read_metadata():
+        with meta_path.open('r') as metafile:
+            return rapidjson.load(metafile)
+
+    rc = client_patch(client, f"{BASE_URI}/backtest/history/randomFile.json")
+    assert_response(rc, 503)
+
+    ftbot.config['user_data_dir'] = tmp_path
+    ftbot.config['runmode'] = RunMode.WEBSERVER
+
+    rc = client_patch(client, f"{BASE_URI}/backtest/history/randomFile.json", {
+        "strategy": CURRENT_TEST_STRATEGY,
+    })
+    assert rc.status_code == 404
+
+    # Nonexisting strategy
+    rc = client_patch(client, f"{BASE_URI}/backtest/history/{file_path.name}", {
+        "strategy": f"{CURRENT_TEST_STRATEGY}xxx",
+    })
+    assert rc.status_code == 400
+    assert rc.json()['detail'] == 'Strategy not in metadata.'
+
+    # no Notes
+    rc = client_patch(client, f"{BASE_URI}/backtest/history/{file_path.name}", {
+        "strategy": CURRENT_TEST_STRATEGY,
+    })
+    assert rc.status_code == 200
+    res = rc.json()
+    assert isinstance(res, list)
+    assert len(res) == 1
+    assert res[0]['strategy'] == CURRENT_TEST_STRATEGY
+    assert res[0]['notes'] == ''
+
+    fileres = read_metadata()
+    assert fileres[CURRENT_TEST_STRATEGY]['run_id'] == res[0]['run_id']
+    assert fileres[CURRENT_TEST_STRATEGY]['notes'] == ''
+
+    rc = client_patch(client, f"{BASE_URI}/backtest/history/{file_path.name}", {
+        "strategy": CURRENT_TEST_STRATEGY,
+        "notes": "FooBar",
+    })
+    assert rc.status_code == 200
+    res = rc.json()
+    assert isinstance(res, list)
+    assert len(res) == 1
+    assert res[0]['strategy'] == CURRENT_TEST_STRATEGY
+    assert res[0]['notes'] == 'FooBar'
+
+    fileres = read_metadata()
+    assert fileres[CURRENT_TEST_STRATEGY]['run_id'] == res[0]['run_id']
+    assert fileres[CURRENT_TEST_STRATEGY]['notes'] == 'FooBar'
+
+
 def test_health(botclient):
     ftbot, client = botclient