[AI][FIX] 피쳐 계산 코드 변경#296
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAI 특성 처리, 트랜스포머 신호 모델 래퍼, 포트폴리오 할당 로직, 일일 파이프라인 루틴을 업데이트했습니다. 앙상블 기반 다중 모델 추론, 데이터베이스 저장소 통합, 스키마 테이블스페이스 재구성을 도입했습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant DataLoader
participant ModelWrappers
participant GatingModel
participant PortfolioRepo
participant MarketData
Client->>DataLoader: Load ticker/sector metadata
DataLoader->>DataLoader: Load ticker_to_sector_id mapping
Client->>MarketData: Fetch market state (OHLCV, VIX)
MarketData-->>Client: Market data dict
Client->>ModelWrappers: For each model wrapper
ModelWrappers->>ModelWrappers: Apply scaler to DataFrame
ModelWrappers->>ModelWrappers: Extract features, pass ticker_id/sector_id
ModelWrappers-->>Client: Per-horizon predictions
Client->>GatingModel: Compute gating weights (2D market state)
GatingModel-->>Client: Model weights vector
Client->>Client: Aggregate signals using weights & gating
Client->>PortfolioRepo: Get current positions/cash
PortfolioRepo-->>Client: Position snapshot
Client->>Client: Apply VIX-based risk overlay (brake logic)
Client->>Client: Calculate final target allocation
Client->>PortfolioRepo: Save portfolio summary & positions
PortfolioRepo-->>Client: Confirmation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
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 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]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@AI/AI/config/watchlist.json`:
- Around line 1-34: The watchlist JSON was added under
AI/AI/config/watchlist.json but DynamicScreener reads AI/config/watchlist.json
at runtime; move the file to the runtime path (AI/config/watchlist.json) or
update DynamicScreener's config path to point to AI/AI/config/watchlist.json.
Specifically, either relocate the file to the expected location so
DynamicScreener loads it, or change the path constant/loader in DynamicScreener
to reference the new location and update any documentation/tests accordingly.
In `@AI/modules/features/market_derived.py`:
- Line 4: Create two empty package initializer files so Python treats the
directories as packages: add an empty AI/modules/__init__.py and an empty
AI/modules/features/__init__.py. This will allow the relative import in
AI/modules/features/market_derived.py (the "from .technical import compute_rsi,
compute_atr, compute_macd, compute_bollinger_bands" statement) to resolve and
also enable the absolute import used in
AI/modules/signal/core/dataset_builder.py ("from
AI.modules.features.market_derived import ...") to succeed.
In `@AI/modules/features/processor.py`:
- Around line 29-32: The code passes a bare pd.Series() as the default for
event_info['ipo_dates'], violating add_date_distance's contract that expects a
datetime Series aligned to the DataFrame index; change the default to an empty
datetime Series indexed to self.df (e.g. a Series with dtype datetime64[ns] and
index=self.df.index) when calling add_date_distance so
add_date_distance(self.df, ipo_dates, 'ipo') always receives an index-aligned
Series; keep the existing call to add_event_window_flags for fomc_dates as-is.
In `@AI/modules/signal/models/transformer/wrapper.py`:
- Around line 86-129: The predict method in this wrapper breaks the
BaseSignalModel contract by returning a Dict when given a DataFrame (violating
BaseSignalModel.predict which must return an np.ndarray), causing
.update(preds_dict) usage in portfolio_logic.py to be inconsistent; fix by
making TransformerSignalModel.predict always return an np.ndarray (e.g., convert
the 4-class dict into a 1x4 numpy array in predict) or alternatively extract the
dict-returning logic into a new method (e.g., predict_with_labels) and keep
predict(...) returning an np.ndarray; update references to use
predict_with_labels where labeled dicts are required and ensure
predict([tensor_data, t_id_tensor, s_id_tensor]) and the legacy ndarray branch
both return np.ndarray to restore LSP compliance.
In `@AI/modules/trader/strategies/portfolio_logic.py`:
- Around line 17-18: The function signature currently declares sector_ids as a
sector->id map but the code at usage (e.g., in portfolio logic where the code
looks up sector_ids[ticker]) expects a ticker->sector_id map; update the
parameter to a clearly named ticker_to_sector_id (or ticker_sector_ids) and its
type/contract to Dict[str, int] mapping ticker -> sector_id, update any
references inside the function (e.g., the lookup at the place currently using
sector_ids[ticker]) to use the new name, and modify callers to pass the correct
ticker->sector_id mapping (or adjust the loader to provide
loader.ticker_to_sector_id instead of loader.sector_to_id); also update the
function docstring/type hints to reflect the new contract so the mismatch is
fixed.
In `@AI/pipelines/daily_routine.py`:
- Around line 296-312: The persisted execution records show "cash_after" always
zero because the dict uses a constant/incorrect value instead of the computed
post-trade cash; update the code that builds the execution payload so
"cash_after" is assigned the actual computed balance variable (e.g., next_cash)
after applying the trade (previous_cash - current_price*qty - commission) and
ensure that the function save_executions_to_db() receives that computed value;
locate the calculation for next_cash/next_qty/next_avg_price in the same routine
and move or recompute next_cash before constructing the execution dict so the
stored "cash_after" reflects real cash flow.
- Around line 85-88: The new ensemble path initializes model_wrappers but still
constructs input features using legacy.technical_features, so updated features
(correlation_spike, recent_loss_ema, IPO/FOMC from event_features.py and
FeatureProcessor in AI/modules/features/processor.py) never reach the models;
replace the legacy feature construction with an invocation of FeatureProcessor
(or call its transform/process method) to produce the df used by the
ensemble/portfolio stage and pass that processed df into model_wrappers (apply
the same change for the other occurrence noted around the later block), ensuring
the FeatureProcessor output columns include the new features before any
inference call.
- Around line 112-125: The pipeline currently hardcodes test artifacts
(tests/multi_horizon_model_test.keras and tests/multi_horizon_scaler_test.pkl)
and always uses transformer_wrapper.model.load_weights(weights_path) regardless
of runtime mode; update the code that sets weights_path (and related scaler
path) to conditionalize on the runtime mode (e.g., a mode variable or
environment flag) so that in 'test' mode it uses the test paths and in
'live'/'prod' mode it uses production artifact locations, and apply the same
change for the similar logic referenced at lines 132-138; ensure the fallback
logic using temp_h5_path and the HDF5 copy remains unchanged but uses the
mode-selected weights_path.
In `@AI/tests/trading_log.py`:
- Around line 33-49: The queries in plot_ticker_trades() build SQL via f-strings
(price_query and trade_query) causing SQL injection; change both to
parameterized queries instead of string interpolation (e.g., use parameter
placeholders in the SQL and pass the ticker via the params argument to
pd.read_sql or use SQLAlchemy text() with parameters), update the two
pd.read_sql calls to supply the params (or a dict) rather than embedding
{ticker}, and ensure the date literal remains constant rather than
user-provided.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1bc5606d-36bc-4d67-8570-ae6a5c815933
📒 Files selected for processing (9)
AI/AI/config/watchlist.jsonAI/modules/features/event_features.pyAI/modules/features/market_derived.pyAI/modules/features/processor.pyAI/modules/signal/models/transformer/wrapper.pyAI/modules/trader/strategies/portfolio_logic.pyAI/pipelines/daily_routine.pyAI/tests/check_positions.pyAI/tests/trading_log.py
💤 Files with no reviewable changes (1)
- AI/tests/check_positions.py
AI/AI/config/watchlist.json
Outdated
Comment on lines
+1
to
+34
| { | ||
| "tickers": [ | ||
| "NVDA", | ||
| "TSLA", | ||
| "MSFT", | ||
| "MU", | ||
| "AAPL", | ||
| "AMZN", | ||
| "GOOGL", | ||
| "AVGO", | ||
| "PLTR", | ||
| "AMD", | ||
| "META", | ||
| "NFLX", | ||
| "GOOG", | ||
| "ORCL", | ||
| "INTC", | ||
| "JPM", | ||
| "XOM", | ||
| "CRM", | ||
| "LLY", | ||
| "COIN", | ||
| "BKNG", | ||
| "INTU", | ||
| "AMAT", | ||
| "APP", | ||
| "WMT", | ||
| "GS", | ||
| "V", | ||
| "LRCX", | ||
| "BAC", | ||
| "UNH" | ||
| ] | ||
| } No newline at end of file |
There was a problem hiding this comment.
실사용 경로와 다른 위치에 watchlist가 추가되었습니다.
DynamicScreener가 갱신하는 경로는 AI/config/watchlist.json인데, 이번 파일은 AI/AI/config/watchlist.json 아래에 있어 런타임에서 사용되지 않습니다. 기본 watchlist를 커밋하려는 의도라면 실제 경로로 옮겨야 합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@AI/AI/config/watchlist.json` around lines 1 - 34, The watchlist JSON was
added under AI/AI/config/watchlist.json but DynamicScreener reads
AI/config/watchlist.json at runtime; move the file to the runtime path
(AI/config/watchlist.json) or update DynamicScreener's config path to point to
AI/AI/config/watchlist.json. Specifically, either relocate the file to the
expected location so DynamicScreener loads it, or change the path
constant/loader in DynamicScreener to reference the new location and update any
documentation/tests accordingly.
| import pandas as pd | ||
| import numpy as np | ||
| from features.technical import compute_rsi, compute_atr, compute_macd, compute_bollinger_bands | ||
| from .technical import compute_rsi, compute_atr, compute_macd, compute_bollinger_bands |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: 패키지 구조를 위한 __init__.py 파일 존재 여부 확인
echo "=== Checking for __init__.py files ==="
echo ""
# Check AI/modules/__init__.py
if [ -f "AI/modules/__init__.py" ]; then
echo "✓ AI/modules/__init__.py exists"
else
echo "✗ AI/modules/__init__.py NOT FOUND (Required for package structure)"
fi
# Check AI/modules/features/__init__.py
if [ -f "AI/modules/features/__init__.py" ]; then
echo "✓ AI/modules/features/__init__.py exists"
else
echo "✗ AI/modules/features/__init__.py NOT FOUND (Required for relative imports)"
fi
echo ""
echo "=== Checking if technical.py exists ==="
if [ -f "AI/modules/features/technical.py" ]; then
echo "✓ AI/modules/features/technical.py exists"
else
echo "✗ AI/modules/features/technical.py NOT FOUND"
fi
echo ""
echo "=== Searching for files that import from market_derived.py ==="
rg -n "from AI\.modules\.features\.market_derived import|from \.market_derived import" --type py -C 1Repository: SISC-IT/sisc-web
Length of output: 944
__init__.py 파일이 누락되어 상대 임포트가 작동하지 않습니다.
상대 임포트(.technical)를 사용하려면 AI/modules/ 와 AI/modules/features/ 디렉토리에 __init__.py 파일이 필요합니다. 현재 두 파일 모두 존재하지 않으므로 ImportError가 발생합니다.
또한 AI/modules/signal/core/dataset_builder.py의 절대 임포트(from AI.modules.features.market_derived import ...)도 패키지 구조가 완성되지 않으면 실패합니다.
다음 파일들을 생성하세요:
AI/modules/__init__.py(빈 파일)AI/modules/features/__init__.py(빈 파일)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@AI/modules/features/market_derived.py` at line 4, Create two empty package
initializer files so Python treats the directories as packages: add an empty
AI/modules/__init__.py and an empty AI/modules/features/__init__.py. This will
allow the relative import in AI/modules/features/market_derived.py (the "from
.technical import compute_rsi, compute_atr, compute_macd,
compute_bollinger_bands" statement) to resolve and also enable the absolute
import used in AI/modules/signal/core/dataset_builder.py ("from
AI.modules.features.market_derived import ...") to succeed.
Comment on lines
+29
to
+32
| # 3. 이벤트 기반 피처 | ||
| if event_info: | ||
| self.df = add_event_features(self.df, event_info) | ||
| self.df = add_date_distance(self.df, event_info.get('ipo_dates', pd.Series()), 'ipo') | ||
| self.df = add_event_window_flags(self.df, event_info.get('fomc_dates', []), 'fomc') |
There was a problem hiding this comment.
ipo_dates 기본값이 helper 입력 계약을 깨고 있습니다.
add_date_distance()는 각 행과 정렬된 이벤트 날짜 Series를 전제로 하는데, 여기서는 key가 없을 때 길이 0짜리 pd.Series()를 넘기고 있습니다. event_info에 fomc_dates만 있고 ipo_dates가 없으면 인덱스가 맞지 않아 잘못된 계산이나 예외로 이어질 수 있습니다.
제안 수정안
- self.df = add_date_distance(self.df, event_info.get('ipo_dates', pd.Series()), 'ipo')
+ ipo_dates = event_info.get('ipo_dates')
+ if ipo_dates is None:
+ ipo_dates = pd.Series(pd.NaT, index=self.df.index, dtype='datetime64[ns]')
+ self.df = add_date_distance(self.df, ipo_dates, 'ipo')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@AI/modules/features/processor.py` around lines 29 - 32, The code passes a
bare pd.Series() as the default for event_info['ipo_dates'], violating
add_date_distance's contract that expects a datetime Series aligned to the
DataFrame index; change the default to an empty datetime Series indexed to
self.df (e.g. a Series with dtype datetime64[ns] and index=self.df.index) when
calling add_date_distance so add_date_distance(self.df, ipo_dates, 'ipo') always
receives an index-aligned Series; keep the existing call to
add_event_window_flags for fomc_dates as-is.
AI/pipelines/daily_routine.py
Outdated
Comment on lines
+85
to
+88
| # ========================================================================= | ||
| # 2. 다중 모델(Wrappers) 로드 및 초기화 (신규 메타 앙상블 로직) | ||
| # ========================================================================= | ||
| model_wrappers = {} # 포트폴리오 로직에 넘길 모델 보관함 |
There was a problem hiding this comment.
새 앙상블 경로가 이번 feature 수정 모듈을 전혀 타지 않습니다.
여기서 래퍼/포트폴리오 단계는 새 흐름으로 바뀌었지만, 실제 입력 df는 아래 전처리 단계에서 여전히 legacy.technical_features로 만들어집니다. 그래서 AI/modules/features/processor.py와 event_features.py에서 바꾼 correlation_spike, recent_loss_ema, IPO/FOMC feature가 추론 경로에 반영되지 않습니다. 이번 PR 목적이 피처 계산 수정이라면 여기서 FeatureProcessor를 연결해야 합니다.
Also applies to: 195-208
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@AI/pipelines/daily_routine.py` around lines 85 - 88, The new ensemble path
initializes model_wrappers but still constructs input features using
legacy.technical_features, so updated features (correlation_spike,
recent_loss_ema, IPO/FOMC from event_features.py and FeatureProcessor in
AI/modules/features/processor.py) never reach the models; replace the legacy
feature construction with an invocation of FeatureProcessor (or call its
transform/process method) to produce the df used by the ensemble/portfolio stage
and pass that processed df into model_wrappers (apply the same change for the
other occurrence noted around the later block), ensuring the FeatureProcessor
output columns include the new features before any inference call.
AI/pipelines/daily_routine.py
Outdated
Comment on lines
112
to
125
| # 1차 시도: 기본 Keras 포맷으로 가중치 로드 | ||
| transformer_wrapper.model.load_weights(weights_path) | ||
| print("✅ [Transformer V1] 모델 가중치 로드 완료 (Standard)") | ||
| except Exception as load_e: | ||
| # 2차 시도: HDF5 Fallback (기존 로직 완벽 보존) | ||
| if "not a zip file" in str(load_e) or "header" in str(load_e): | ||
| #print("⚠️ Zip 포맷 로드 실패. HDF5 방식으로 재시도합니다.") | ||
| # 확장자를 .h5로 변경한 임시 파일 생성 (Keras가 확장자를 보고 로더를 결정함) | ||
| temp_h5_path = weights_path.replace(".keras", "_temp_fallback.h5") | ||
| try: | ||
| shutil.copyfile(weights_path, temp_h5_path) | ||
| model.load_weights(temp_h5_path) | ||
| #print("✅ AI 멀티 호라이즌 모델 로드 완료 (HDF5 Fallback)") | ||
| transformer_wrapper.model.load_weights(temp_h5_path) | ||
| print("✅ [Transformer V1] 모델 가중치 로드 완료 (HDF5 Fallback)") | ||
| except Exception as e_h5: | ||
| print(f"❌ HDF5 로드 실패했습니다: {e_h5}") | ||
| print(f"❌ HDF5 로드 실패: {e_h5}") | ||
| raise e_h5 |
There was a problem hiding this comment.
daily pipeline이 테스트 아티팩트를 기본 모델로 로드합니다.
로드 경로가 tests/multi_horizon_model_test.keras와 tests/multi_horizon_scaler_test.pkl로 고정돼 있고, 현재는 mode와도 분리되지 않습니다. 이 상태면 simulation/live 결과가 검증용 산출물에 묶이니, 운영 경로와 테스트 경로를 분리해야 합니다.
Also applies to: 132-138
🧰 Tools
🪛 Ruff (0.15.5)
[warning] 125-125: Use raise without specifying exception name
Remove exception name
(TRY201)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@AI/pipelines/daily_routine.py` around lines 112 - 125, The pipeline currently
hardcodes test artifacts (tests/multi_horizon_model_test.keras and
tests/multi_horizon_scaler_test.pkl) and always uses
transformer_wrapper.model.load_weights(weights_path) regardless of runtime mode;
update the code that sets weights_path (and related scaler path) to
conditionalize on the runtime mode (e.g., a mode variable or environment flag)
so that in 'test' mode it uses the test paths and in 'live'/'prod' mode it uses
production artifact locations, and apply the same change for the similar logic
referenced at lines 132-138; ensure the fallback logic using temp_h5_path and
the HDF5 copy remains unchanged but uses the mode-selected weights_path.
Comment on lines
+33
to
+49
| price_query = f""" | ||
| SELECT date, close | ||
| FROM public.price_data | ||
| WHERE ticker = '{ticker}' | ||
| AND date >= '2025-08-01' | ||
| ORDER BY date ASC; | ||
| """ | ||
| df_price = pd.read_sql(price_query, conn) | ||
|
|
||
| # 2. 해당 종목의 모든 체결 내역 조회 | ||
| trade_query = f""" | ||
| SELECT fill_date, fill_price, side, qty | ||
| FROM public.executions | ||
| WHERE ticker = '{ticker}' | ||
| ORDER BY fill_date ASC; | ||
| """ | ||
| df_trades = pd.read_sql(trade_query, conn) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and examine the file
find . -name "trading_log.py" -type fRepository: SISC-IT/sisc-web
Length of output: 85
🏁 Script executed:
# Check the file content around lines 33-49
if [ -f "AI/tests/trading_log.py" ]; then
cat -n "AI/tests/trading_log.py" | sed -n '30,55p'
fiRepository: SISC-IT/sisc-web
Length of output: 983
🌐 Web query:
pandas read_sql parameter binding syntax %s placeholders
💡 Result:
pandas.read_sql() / read_sql_query() just forwards your SQL + params to the underlying driver, so the placeholder syntax is driver-dependent (PEP 249 paramstyle). (pandas.pydata.org)
If your driver uses %s (PEP 249 format)
Use positional placeholders and pass a tuple/list:
sql = "SELECT * FROM my_table WHERE id = %s AND status = %s"
df = pd.read_sql(sql, con, params=(id_val, status_val))PEP 249 defines this as format: ... WHERE name=%s. (peps.python.org)
Psycopg (PostgreSQL) note
Psycopg uses %s placeholders with a sequence of parameters; don’t use Python string interpolation to fill them. (psycopg.org)
If you’re using SQLAlchemy text() (common with read_sql)
Prefer named binds like :name and pass a dict; SQLAlchemy will translate to the DBAPI’s actual paramstyle:
from sqlalchemy import text
sql = text("SELECT * FROM my_table WHERE id = :id")
df = pd.read_sql(sql, con, params={"id": id_val})Pandas documents this pattern and reiterates that supported styles depend on the driver/PEP 249. (pandas.pydata.org)
Citations:
- 1: https://pandas.pydata.org/pandas-docs/stable/reference/api/pandas.read_sql.html
- 2: https://peps.python.org/pep-0249/
- 3: https://www.psycopg.org/psycopg3/docs/basic/params.html?utm_source=openai
- 4: https://pandas.pydata.org/pandas-docs/stable/reference/api/pandas.read_sql.html
🏁 Script executed:
# Check the function signature and whether it's public
cat -n "AI/tests/trading_log.py" | sed -n '1,35p'Repository: SISC-IT/sisc-web
Length of output: 1354
🏁 Script executed:
# Look for database connection setup to identify the driver
rg -A 5 "conn\s*=" "AI/tests/trading_log.py" | head -20Repository: SISC-IT/sisc-web
Length of output: 162
🏁 Script executed:
# Check for imports to identify database driver
cat -n "AI/tests/trading_log.py" | sed -n '1,30p'Repository: SISC-IT/sisc-web
Length of output: 1158
🏁 Script executed:
# Search for psycopg or other database driver usage in the project
rg "psycopg|psycopg2|psycopg3|sqlite|mysql" --type py -i | head -20Repository: SISC-IT/sisc-web
Length of output: 1058
SQL injection 취약점 - 양쪽 쿼리 모두 파라미터 바인딩으로 수정하세요.
plot_ticker_trades() 함수는 공개 인터페이스로서 ticker 입력을 신뢰할 수 없으며, 현재 두 SQL 쿼리 모두 f-string으로 직접 삽입하고 있습니다. 작은 따옴표나 SQL 문법을 포함한 입력으로 WHERE 절을 탈출할 수 있으므로 파라미터 바인딩을 사용해야 합니다.
수정안
- price_query = f"""
+ price_query = """
SELECT date, close
FROM public.price_data
- WHERE ticker = '{ticker}'
- AND date >= '2025-08-01'
+ WHERE ticker = %s
+ AND date >= %s
ORDER BY date ASC;
"""
- df_price = pd.read_sql(price_query, conn)
+ df_price = pd.read_sql(price_query, conn, params=(ticker, "2025-08-01"))
- trade_query = f"""
+ trade_query = """
SELECT fill_date, fill_price, side, qty
FROM public.executions
- WHERE ticker = '{ticker}'
+ WHERE ticker = %s
ORDER BY fill_date ASC;
"""
- df_trades = pd.read_sql(trade_query, conn)
+ df_trades = pd.read_sql(trade_query, conn, params=(ticker,))🧰 Tools
🪛 Ruff (0.15.5)
[error] 33-39: Possible SQL injection vector through string-based query construction
(S608)
[error] 43-48: Possible SQL injection vector through string-based query construction
(S608)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@AI/tests/trading_log.py` around lines 33 - 49, The queries in
plot_ticker_trades() build SQL via f-strings (price_query and trade_query)
causing SQL injection; change both to parameterized queries instead of string
interpolation (e.g., use parameter placeholders in the SQL and pass the ticker
via the params argument to pd.read_sql or use SQLAlchemy text() with
parameters), update the two pd.read_sql calls to supply the params (or a dict)
rather than embedding {ticker}, and ensure the date literal remains constant
rather than user-provided.
There was a problem hiding this comment.
🧹 Nitpick comments (6)
AI/pipelines/daily_routine.py (4)
200-208: 미사용 변수all_signals_map처리가 필요합니다.
all_signals_map이 언팩되지만 사용되지 않습니다. 향후 사용 계획이 있다면 언더스코어 접두사(_all_signals_map)를 붙여 의도적인 미사용임을 명시하세요.제안 수정안
- target_weights, scores, all_signals_map = calculate_portfolio_allocation( + target_weights, scores, _all_signals_map = calculate_portfolio_allocation(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@AI/pipelines/daily_routine.py` around lines 200 - 208, The unpacked return value all_signals_map from calculate_portfolio_allocation is not used; to mark it as intentionally unused, rename it to _all_signals_map (or prefix with an underscore) in the assignment where calculate_portfolio_allocation is called so linters/readers recognize it's intentionally discarded; keep the other variables (target_weights, scores) unchanged and do not remove the calculate_portfolio_allocation call.
123-130: 예외 재발생 시raise만 사용하세요.
raise e_h5와raise load_e대신 bareraise를 사용하면 원래 트레이스백이 보존됩니다.제안 수정안
except Exception as e_h5: print(f"❌ HDF5 로드 실패: {e_h5}") - raise e_h5 + raise finally: if os.path.exists(temp_h5_path): os.remove(temp_h5_path) else: - raise load_e + raise🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@AI/pipelines/daily_routine.py` around lines 123 - 130, In the HDF5 loading exception handlers replace the explicit re-raises (raise e_h5 and raise load_e) with a bare "raise" so the original traceback is preserved; update the except block that catches e_h5 (and its finally that cleans up temp_h5_path) and the else branch that currently does "raise load_e" to use bare raise instead.
167-167: 한 줄에 여러 문장이 있습니다.PEP 8 스타일 가이드에 따라
if문과continue를 별도의 줄에 작성하세요.제안 수정안
- if df.empty: continue + if df.empty: + continue🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@AI/pipelines/daily_routine.py` at line 167, 한 줄에 if 문과 continue가 함께 있어 PEP8 위반입니다; AI/pipelines/daily_routine.py에서 현재 "if df.empty: continue"처럼 작성된 조건문을 찾아 if df.empty: 를 한 줄에, 그 다음 줄에 들여쓰기된 continue를 각각 분리해서 작성하도록 수정하세요 (참조 심볼: df.empty 및 해당 if 문 블록).
328-328:zip()에strict=True를 추가하세요.
report_results와saved_report_ids의 길이가 다를 경우 데이터 불일치가 조용히 발생할 수 있습니다.strict=True를 추가하면 길이 불일치 시 즉시 예외가 발생하여 버그를 조기에 발견할 수 있습니다.제안 수정안
- saved_report_map = {r["ticker"]: saved_id for r, saved_id in zip(report_results, saved_report_ids)} + saved_report_map = {r["ticker"]: saved_id for r, saved_id in zip(report_results, saved_report_ids, strict=True)}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@AI/pipelines/daily_routine.py` at line 328, The zip between report_results and saved_report_ids used to build saved_report_map should be made strict to surface length mismatches: update the expression that creates saved_report_map (the assignment using saved_report_map, report_results and saved_report_ids) to call zip with strict=True so a ValueError is raised if the iterables differ in length.AI/modules/features/processor.py (2)
34-36: 불필요한 주석을 제거해 주세요.
"사용자님이 완벽하게 고치신 부분! 👍"주석은 프로덕션 코드에 적합하지 않습니다. 필요하다면 기능을 설명하는 기술적인 주석으로 교체하세요.제안 수정안
- # 사용자님이 완벽하게 고치신 부분! 👍 + # 4. VIX 상관계수 스파이크 및 손실 EMA 계산 if 'vix_close' in self.df.columns: self.df['correlation_spike'] = compute_correlation_spike(self.df['close'], self.df['vix_close'])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@AI/modules/features/processor.py` around lines 34 - 36, Remove the informal/commentary line "사용자님이 완벽하게 고치신 부분! 👍" and replace it with a concise, technical comment (or remove entirely) that explains the intent of the conditional: checking for the 'vix_close' column and computing a 'correlation_spike' column via compute_correlation_spike(self.df['close'], self.df['vix_close']) so locate the block that references self.df, the 'vix_close' check, and compute_correlation_spike and update the comment to be production-appropriate.
20-20: 미사용 파라미터sector_df처리가 필요합니다.
sector_df파라미터가 선언되어 있지만 사용되지 않습니다. 향후 구현 예정이라면 언더스코어 접두사(_sector_df)를 붙이거나, 사용 계획이 없다면 제거해 주세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@AI/modules/features/processor.py` at line 20, The execute_pipeline method declares an unused parameter sector_df; either remove the parameter from the signature or rename it to indicate intentional non-use (e.g., _sector_df) and update any callers accordingly. Locate the method execute_pipeline in class/module AI/modules/features/processor.py and either delete the sector_df parameter from the def execute_pipeline(...) signature and from all calls, or change it to _sector_df to suppress linter warnings and leave a short todo comment if it will be used later.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@AI/modules/features/processor.py`:
- Around line 34-36: Remove the informal/commentary line "사용자님이 완벽하게 고치신 부분! 👍"
and replace it with a concise, technical comment (or remove entirely) that
explains the intent of the conditional: checking for the 'vix_close' column and
computing a 'correlation_spike' column via
compute_correlation_spike(self.df['close'], self.df['vix_close']) so locate the
block that references self.df, the 'vix_close' check, and
compute_correlation_spike and update the comment to be production-appropriate.
- Line 20: The execute_pipeline method declares an unused parameter sector_df;
either remove the parameter from the signature or rename it to indicate
intentional non-use (e.g., _sector_df) and update any callers accordingly.
Locate the method execute_pipeline in class/module
AI/modules/features/processor.py and either delete the sector_df parameter from
the def execute_pipeline(...) signature and from all calls, or change it to
_sector_df to suppress linter warnings and leave a short todo comment if it will
be used later.
In `@AI/pipelines/daily_routine.py`:
- Around line 200-208: The unpacked return value all_signals_map from
calculate_portfolio_allocation is not used; to mark it as intentionally unused,
rename it to _all_signals_map (or prefix with an underscore) in the assignment
where calculate_portfolio_allocation is called so linters/readers recognize it's
intentionally discarded; keep the other variables (target_weights, scores)
unchanged and do not remove the calculate_portfolio_allocation call.
- Around line 123-130: In the HDF5 loading exception handlers replace the
explicit re-raises (raise e_h5 and raise load_e) with a bare "raise" so the
original traceback is preserved; update the except block that catches e_h5 (and
its finally that cleans up temp_h5_path) and the else branch that currently does
"raise load_e" to use bare raise instead.
- Line 167: 한 줄에 if 문과 continue가 함께 있어 PEP8 위반입니다;
AI/pipelines/daily_routine.py에서 현재 "if df.empty: continue"처럼 작성된 조건문을 찾아 if
df.empty: 를 한 줄에, 그 다음 줄에 들여쓰기된 continue를 각각 분리해서 작성하도록 수정하세요 (참조 심볼: df.empty 및
해당 if 문 블록).
- Line 328: The zip between report_results and saved_report_ids used to build
saved_report_map should be made strict to surface length mismatches: update the
expression that creates saved_report_map (the assignment using saved_report_map,
report_results and saved_report_ids) to call zip with strict=True so a
ValueError is raised if the iterables differ in length.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3b6c7ed3-9a54-4ff3-88e3-1124d80f62b5
📒 Files selected for processing (6)
AI/AI/config/watchlist.jsonAI/modules/__init__.pyAI/modules/features/__init__.pyAI/modules/features/legacy/technical_features.pyAI/modules/features/processor.pyAI/pipelines/daily_routine.py
💤 Files with no reviewable changes (1)
- AI/modules/features/legacy/technical_features.py
🚧 Files skipped from review as they are similar to previous changes (1)
- AI/AI/config/watchlist.json
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (1)
AI/pipelines/daily_routine.py (1)
110-112:⚠️ Potential issue | 🟠 Major테스트 아티팩트가 프로덕션 경로로 사용됨
모델 가중치와 스케일러 경로가
tests/디렉토리의 테스트 아티팩트로 하드코딩되어 있습니다.mode파라미터(simulation/live)에 따라 경로를 분기해야 합니다.♻️ 제안 수정안
weights_dir = os.path.join(project_root, "AI/data/weights/transformer") - weights_path = os.path.join(weights_dir, "tests/multi_horizon_model_test.keras") - scaler_path = os.path.join(weights_dir, "tests/multi_horizon_scaler_test.pkl") + if mode == "live": + weights_path = os.path.join(weights_dir, "production/multi_horizon_model.keras") + scaler_path = os.path.join(weights_dir, "production/multi_horizon_scaler.pkl") + else: + weights_path = os.path.join(weights_dir, "tests/multi_horizon_model_test.keras") + scaler_path = os.path.join(weights_dir, "tests/multi_horizon_scaler_test.pkl")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@AI/pipelines/daily_routine.py` around lines 110 - 112, The current code hardcodes test artifact paths into weights_path and scaler_path (variables weights_dir, weights_path, scaler_path); change this to branch on the function/class parameter mode (expected values "simulation" or "live") and select test artifacts only when mode == "simulation" (e.g., join weights_dir with tests/...); for "live" use the production filenames/location (e.g., a prod or default subdirectory or canonical filenames under weights_dir) and validate existence, raising a clear error if missing; update any callers of the function that rely on these variables to pass mode if not already.
🧹 Nitpick comments (2)
AI/pipelines/daily_routine.py (1)
168-168: 한 줄에 여러 문장 작성 (E701)스타일 가이드 위반입니다. 가독성을 위해 분리하세요.
♻️ 제안 수정안
- if df.empty: continue + if df.empty: + continue🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@AI/pipelines/daily_routine.py` at line 168, The line "if df.empty: continue" violates E701 by combining statements on one line; locate the loop where the DataFrame variable "df" is checked in AI/pipelines/daily_routine.py and refactor that check into two separate lines (an "if df.empty:" line followed by an indented "continue" line) so the conditional and the action are on separate lines; ensure indentation matches the surrounding loop context.AI/libs/database/repository.py (1)
15-15: PEP 484: 명시적Optional타입 사용 권장
target_date: str = None은 암묵적Optional입니다. Python 타입 힌트 명세에 따라str | None으로 명시하는 것이 좋습니다.♻️ 제안 수정안
-def get_current_position(ticker: str, target_date: str = None, initial_cash: float = 10000000, db_name: str = "db") -> dict: +def get_current_position(ticker: str, target_date: str | None = None, initial_cash: float = 10000000, db_name: str = "db") -> dict:Line 94도 동일하게 적용:
-def get_current_cash(target_date: str = None, initial_cash: float = 10000000, db_name: str = "db") -> float: +def get_current_cash(target_date: str | None = None, initial_cash: float = 10000000, db_name: str = "db") -> float:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@AI/libs/database/repository.py` at line 15, Change the implicit Optional type annotation on the get_current_position signature so the target_date parameter is explicitly optional (use typing.Optional[str] or the union syntax str | None) instead of target_date: str = None; also apply the same fix to the other function signature in this module that currently uses target_date: str = None so all optional date params are explicitly annotated.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@AI/libs/database/repository.py`:
- Around line 94-124: In get_current_cash, handle the case where target_date is
None before executing the SQL to avoid the "WHERE date < NULL" comparison:
branch on target_date and, if None, run a query that selects the latest cash row
without the "WHERE date < %s" filter (no parameterized arg), otherwise keep the
existing parameterized query and call cursor.execute(query, (target_date,)).
Update the code paths around cursor.execute/result handling so cursor.execute is
called with the correct SQL and parameters depending on target_date and ensure
cursor.close()/conn.close() remain in finally.
In `@AI/modules/signal/core/data_loader.py`:
- Line 50: You renamed the DataLoader attribute from ticker_sector_map to
ticker_sector_id but left call sites referencing the old name; update all uses
of loader.ticker_sector_map.get(ticker, 0) to
loader.ticker_sector_id.get(ticker, 0) (and any other direct accesses to
ticker_sector_map) so they reference the new attribute name (ticker_sector_id)
to avoid AttributeError at runtime.
In `@AI/modules/trader/strategies/portfolio_logic.py`:
- Around line 81-86: The base_signal_vector is built using model_names (e.g.,
"transformer_v1") but signals from TransformerSignalModel.predict have
horizon-suffixed keys (e.g., "transformer_v1_1d"), causing defaults of 0.5;
update the vector construction in the code that builds base_signal_vector to
resolve names to the actual signal keys—either by looking up a specific horizon
(e.g., signals.get(f"{name}_1d", 0.5)) or by collecting and averaging all
signals whose keys start with the model name prefix (filter keys starting with
name and take mean, falling back to 0.5 if none) so that base_signal_vector and
subsequent np.dot with model_weights use real predictions from
TransformerSignalModel.predict.
- Around line 46-56: The predict call uses keyword args only supported by
TransformerSignalModel, causing TypeError for
BaseSignalModel/TCNWrapper/PatchTSTWrapper/iTransformerWrapper; change the call
in the loop to try the transformer-style signature first (wrapper.predict(df,
ticker_id=t_id, sector_id=s_id)) and on TypeError or unexpected signature
fallback to the simple positional call (wrapper.predict(df) or
wrapper.predict(X_input)) so all wrappers are compatible; additionally normalize
transformer return keys by detecting keys starting with "transformer" (e.g.,
"transformer_1d","transformer_3d") from TransformerSignalModel and aggregate or
map them to a canonical "transformer" key (e.g., average the transformer_*
scores or pick the appropriate subkey) so later lookup that uses
signals.get("transformer", 0.5) finds the transformer signal.
In `@AI/pipelines/daily_routine.py`:
- Around line 204-206: The code references a non-existent attribute
loader.ticker_to_sector_id causing an AttributeError; update the call site in
daily_routine.py (the function passing ticker_ids=loader.ticker_to_id,
ticker_to_sector_id=...) to use the correct DataLoader attribute
loader.ticker_sector_id instead of loader.ticker_to_sector_id, and scan for any
other uses of ticker_to_sector_id to replace them or adapt parameter names
accordingly.
- Around line 46-47: The code imports and instantiates a non-existent class
PortfolioRepository and then calls repo.method() everywhere; instead revert to
using the module-level functions defined in AI.libs.database.repository (e.g.,
get_current_position, get_current_cash, save_executions_to_db, etc.): remove the
PortfolioRepository import/instantiation (remove repo =
PortfolioRepository(...)) and replace every repo.get_current_cash(),
repo.get_current_position(), repo.save_executions_to_db(), etc. with direct
calls to get_current_cash(), get_current_position(), save_executions_to_db()
(and import those functions from AI.libs.database.repository) so all calls match
the actual function names in the module.
---
Duplicate comments:
In `@AI/pipelines/daily_routine.py`:
- Around line 110-112: The current code hardcodes test artifact paths into
weights_path and scaler_path (variables weights_dir, weights_path, scaler_path);
change this to branch on the function/class parameter mode (expected values
"simulation" or "live") and select test artifacts only when mode == "simulation"
(e.g., join weights_dir with tests/...); for "live" use the production
filenames/location (e.g., a prod or default subdirectory or canonical filenames
under weights_dir) and validate existence, raising a clear error if missing;
update any callers of the function that rely on these variables to pass mode if
not already.
---
Nitpick comments:
In `@AI/libs/database/repository.py`:
- Line 15: Change the implicit Optional type annotation on the
get_current_position signature so the target_date parameter is explicitly
optional (use typing.Optional[str] or the union syntax str | None) instead of
target_date: str = None; also apply the same fix to the other function signature
in this module that currently uses target_date: str = None so all optional date
params are explicitly annotated.
In `@AI/pipelines/daily_routine.py`:
- Line 168: The line "if df.empty: continue" violates E701 by combining
statements on one line; locate the loop where the DataFrame variable "df" is
checked in AI/pipelines/daily_routine.py and refactor that check into two
separate lines (an "if df.empty:" line followed by an indented "continue" line)
so the conditional and the action are on separate lines; ensure indentation
matches the surrounding loop context.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5625dbd5-5a29-4019-bae6-f2dc9c37b6e4
📒 Files selected for processing (4)
AI/libs/database/repository.pyAI/modules/signal/core/data_loader.pyAI/modules/trader/strategies/portfolio_logic.pyAI/pipelines/daily_routine.py
AI/libs/database/repository.py
Outdated
Comment on lines
+94
to
+124
| def get_current_cash(target_date: str = None, initial_cash: float = 10000000, db_name: str = "db") -> float: | ||
| """ | ||
| [현재 포트폴리오 현금 조회] (업그레이드: 미래 데이터 차단) | ||
| target_date 이전까지의 체결 내역을 기반으로 정확한 현금 잔고를 계산합니다. | ||
| """ | ||
| conn = get_db_conn(db_name) | ||
| if conn is None: | ||
| return initial_cash | ||
|
|
||
| cursor = conn.cursor() | ||
|
|
||
| query = """ | ||
| SELECT cash | ||
| FROM public.portfolio_summary | ||
| WHERE date = (SELECT MAX(date) FROM public.portfolio_summary WHERE date < %s) | ||
| LIMIT 1; | ||
| """ | ||
|
|
||
| try: | ||
| cursor.execute(query, (target_date,)) | ||
| result = cursor.fetchone() | ||
| if result: | ||
| return float(result[0]) | ||
| else: | ||
| return initial_cash | ||
| except Exception as e: | ||
| print(f"[Repository][Error] 포트폴리오 현금 조회 중 오류 발생: {e}") | ||
| return initial_cash | ||
| finally: | ||
| cursor.close() | ||
| conn.close() |
There was a problem hiding this comment.
target_date가 None일 때 SQL 쿼리 오류 발생
target_date가 None으로 전달되면 Line 113에서 cursor.execute(query, (target_date,))가 실행될 때 WHERE date < NULL 비교가 발생하여 예상치 못한 결과(항상 빈 결과)를 반환합니다.
🐛 제안 수정안
def get_current_cash(target_date: str | None = None, initial_cash: float = 10000000, db_name: str = "db") -> float:
"""
[현재 포트폴리오 현금 조회] (업그레이드: 미래 데이터 차단)
target_date 이전까지의 체결 내역을 기반으로 정확한 현금 잔고를 계산합니다.
"""
conn = get_db_conn(db_name)
if conn is None:
return initial_cash
cursor = conn.cursor()
+ if target_date is None:
+ query = """
+ SELECT cash
+ FROM public.portfolio_summary
+ ORDER BY date DESC
+ LIMIT 1;
+ """
+ params = ()
+ else:
+ query = """
+ SELECT cash
+ FROM public.portfolio_summary
+ WHERE date = (SELECT MAX(date) FROM public.portfolio_summary WHERE date < %s)
+ LIMIT 1;
+ """
+ params = (target_date,)
- query = """
- SELECT cash
- FROM public.portfolio_summary
- WHERE date = (SELECT MAX(date) FROM public.portfolio_summary WHERE date < %s)
- LIMIT 1;
- """
try:
- cursor.execute(query, (target_date,))
+ cursor.execute(query, params)📝 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.
Suggested change
| def get_current_cash(target_date: str = None, initial_cash: float = 10000000, db_name: str = "db") -> float: | |
| """ | |
| [현재 포트폴리오 현금 조회] (업그레이드: 미래 데이터 차단) | |
| target_date 이전까지의 체결 내역을 기반으로 정확한 현금 잔고를 계산합니다. | |
| """ | |
| conn = get_db_conn(db_name) | |
| if conn is None: | |
| return initial_cash | |
| cursor = conn.cursor() | |
| query = """ | |
| SELECT cash | |
| FROM public.portfolio_summary | |
| WHERE date = (SELECT MAX(date) FROM public.portfolio_summary WHERE date < %s) | |
| LIMIT 1; | |
| """ | |
| try: | |
| cursor.execute(query, (target_date,)) | |
| result = cursor.fetchone() | |
| if result: | |
| return float(result[0]) | |
| else: | |
| return initial_cash | |
| except Exception as e: | |
| print(f"[Repository][Error] 포트폴리오 현금 조회 중 오류 발생: {e}") | |
| return initial_cash | |
| finally: | |
| cursor.close() | |
| conn.close() | |
| def get_current_cash(target_date: str | None = None, initial_cash: float = 10000000, db_name: str = "db") -> float: | |
| """ | |
| [현재 포트폴리오 현금 조회] (업그레이드: 미래 데이터 차단) | |
| target_date 이전까지의 체결 내역을 기반으로 정확한 현금 잔고를 계산합니다. | |
| """ | |
| conn = get_db_conn(db_name) | |
| if conn is None: | |
| return initial_cash | |
| cursor = conn.cursor() | |
| if target_date is None: | |
| query = """ | |
| SELECT cash | |
| FROM public.portfolio_summary | |
| ORDER BY date DESC | |
| LIMIT 1; | |
| """ | |
| params = () | |
| else: | |
| query = """ | |
| SELECT cash | |
| FROM public.portfolio_summary | |
| WHERE date = (SELECT MAX(date) FROM public.portfolio_summary WHERE date < %s) | |
| LIMIT 1; | |
| """ | |
| params = (target_date,) | |
| try: | |
| cursor.execute(query, params) | |
| result = cursor.fetchone() | |
| if result: | |
| return float(result[0]) | |
| else: | |
| return initial_cash | |
| except Exception as e: | |
| print(f"[Repository][Error] 포트폴리오 현금 조회 중 오류 발생: {e}") | |
| return initial_cash | |
| finally: | |
| cursor.close() | |
| conn.close() |
🧰 Tools
🪛 Ruff (0.15.5)
[warning] 94-94: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
[warning] 119-119: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@AI/libs/database/repository.py` around lines 94 - 124, In get_current_cash,
handle the case where target_date is None before executing the SQL to avoid the
"WHERE date < NULL" comparison: branch on target_date and, if None, run a query
that selects the latest cash row without the "WHERE date < %s" filter (no
parameterized arg), otherwise keep the existing parameterized query and call
cursor.execute(query, (target_date,)). Update the code paths around
cursor.execute/result handling so cursor.execute is called with the correct SQL
and parameters depending on target_date and ensure cursor.close()/conn.close()
remain in finally.
| self.ticker_to_id: Dict[str, int] = {} | ||
| self.sector_to_id: Dict[str, int] = {} | ||
| self.ticker_sector_map: Dict[str, int] = {} | ||
| self.ticker_sector_id: Dict[str, int] = {} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# ticker_sector_map 속성 사용처 검색
rg -n "ticker_sector_map" --type=pyRepository: SISC-IT/sisc-web
Length of output: 275
속성 이름 변경 후 미처리된 호출부로 인한 런타임 오류 발생
ticker_sector_map → ticker_sector_id로 이름을 변경했으나, 다음 파일들에서 여전히 이전 속성명을 참조합니다:
AI/modules/signal/workflows/inference.py:54:loader.ticker_sector_map.get(ticker, 0)AI/modules/signal/workflows/train_calibrator.py:112:loader.ticker_sector_map.get(ticker, 0)
실행 시 AttributeError가 발생합니다. 모든 호출부를 ticker_sector_id로 업데이트하세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@AI/modules/signal/core/data_loader.py` at line 50, You renamed the DataLoader
attribute from ticker_sector_map to ticker_sector_id but left call sites
referencing the old name; update all uses of
loader.ticker_sector_map.get(ticker, 0) to loader.ticker_sector_id.get(ticker,
0) (and any other direct accesses to ticker_sector_map) so they reference the
new attribute name (ticker_sector_id) to avoid AttributeError at runtime.
Comment on lines
+46
to
+56
| for model_name in model_names: | ||
| wrapper = model_wrappers[model_name] | ||
| try: | ||
| # 명세서에 따라 각 모델은 자신만의 필요 데이터(Tech, Log_return, Macro)를 | ||
| # Wrapper 내부에서 추출하여 추론합니다. | ||
| preds_dict = wrapper.predict(df) | ||
| # 각 Wrapper 내부에서 필요 데이터를 전처리하고 추론값을 반환 | ||
| preds_dict = wrapper.predict(df, ticker_id=t_id, sector_id=s_id) | ||
| # preds_dict가 {'TCN': 0.65} 형태라고 가정 | ||
| ticker_signals.update(preds_dict) | ||
| except Exception as e: | ||
| print(f"[{ticker}] {model_name} 추론 에러: {e}") | ||
| continue | ||
| print(f"[Phase 3] [{ticker}] {model_name} 추론 에러: {e}") | ||
| # 에러 발생 시 중립(0.5) 스코어 부여 | ||
| ticker_signals[model_name] = 0.5 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 모든 wrapper의 predict 메서드 시그니처 확인
ast-grep --pattern $'def predict(self, $_) {
$$$
}'Repository: SISC-IT/sisc-web
Length of output: 42
🏁 Script executed:
#!/bin/bash
# 래퍼 클래스 파일 찾기
echo "=== Finding wrapper-related files ==="
fd -e py | grep -E "(wrapper|signal|model)" | head -20
echo -e "\n=== Searching for BaseSignalModel ==="
rg "class BaseSignalModel" -A 5
echo -e "\n=== Searching for TCNWrapper ==="
rg "class TCNWrapper" -A 5
echo -e "\n=== Searching for TransformerSignalModel ==="
rg "class TransformerSignalModel" -A 5Repository: SISC-IT/sisc-web
Length of output: 2269
🏁 Script executed:
#!/bin/bash
# 각 wrapper의 predict 메서드 서명 확인
echo "=== BaseSignalModel predict() ==="
rg "def predict" AI/modules/signal/core/base_model.py -A 3
echo -e "\n=== TCNWrapper predict() ==="
rg "def predict" AI/modules/signal/models/TCN/wrapper.py -A 3
echo -e "\n=== TransformerSignalModel predict() ==="
rg "def predict" AI/modules/signal/models/transformer/wrapper.py -A 3
echo -e "\n=== PatchTSTWrapper predict() ==="
rg "def predict" AI/modules/signal/models/PatchTST/wrapper.py -A 3
echo -e "\n=== iTransformerWrapper predict() ==="
rg "def predict" AI/modules/signal/models/itransformer/wrapper.py -A 3Repository: SISC-IT/sisc-web
Length of output: 1179
🏁 Script executed:
#!/bin/bash
# portfolio_logic.py 파일 크기 확인
echo "=== File size check ==="
wc -l AI/modules/trader/strategies/portfolio_logic.py
echo -e "\n=== Lines 46-85 (context around line 82) ==="
sed -n '46,85p' AI/modules/trader/strategies/portfolio_logic.pyRepository: SISC-IT/sisc-web
Length of output: 1998
🏁 Script executed:
#!/bin/bash
# portfolio_logic.py 전체 파일 검토
echo "=== Full portfolio_logic.py file ==="
cat -n AI/modules/trader/strategies/portfolio_logic.py
echo -e "\n=== TransformerSignalModel predict return structure ==="
rg "def predict" AI/modules/signal/models/transformer/wrapper.py -A 30 | head -50Repository: SISC-IT/sisc-web
Length of output: 7251
🏁 Script executed:
#!/bin/bash
# TransformerSignalModel predict 메서드 전체 확인
echo "=== TransformerSignalModel.predict() complete method ==="
sed -n '23,80p' AI/modules/signal/models/transformer/wrapper.py
echo -e "\n=== Check what keys are returned from TransformerSignalModel ==="
rg "return {" AI/modules/signal/models/transformer/wrapper.py -A 5Repository: SISC-IT/sisc-web
Length of output: 2844
🏁 Script executed:
#!/bin/bash
# TransformerSignalModel predict 메서드 전체 확인 (다른 방식)
echo "=== TransformerSignalModel.predict() method ==="
sed -n '80,150p' AI/modules/signal/models/transformer/wrapper.py
echo -e "\n=== Check return statements in wrapper ==="
rg "return" AI/modules/signal/models/transformer/wrapper.py -B 2 -A 1 | head -40Repository: SISC-IT/sisc-web
Length of output: 3548
모델 래퍼 호환성 문제: ticker_id, sector_id 파라미터 미지원 래퍼 존재
Line 50의 wrapper.predict(df, ticker_id=t_id, sector_id=s_id) 호출은 TransformerSignalModel에서만 지원됩니다. BaseSignalModel.predict()와 TCNWrapper.predict(), PatchTSTWrapper.predict(), iTransformerWrapper.predict()는 X_input 파라미터만 받으므로, 이 모들이 model_wrappers에 포함되면 TypeError: predict() got unexpected keyword arguments 예외가 발생합니다.
추가로, TransformerSignalModel.predict()가 DataFrame을 받으면 {"transformer_1d": 0.65, "transformer_3d": 0.70, ...} 형태로 반환하지만, Line 82에서는 signals.get(name, 0.5)로 "transformer" 키를 조회합니다. 이 키가 존재하지 않아 항상 0.5로 폴백되므로 트랜스포머 모델의 신호가 무시됩니다.
🧰 Tools
🪛 Ruff (0.15.5)
[warning] 53-53: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@AI/modules/trader/strategies/portfolio_logic.py` around lines 46 - 56, The
predict call uses keyword args only supported by TransformerSignalModel, causing
TypeError for BaseSignalModel/TCNWrapper/PatchTSTWrapper/iTransformerWrapper;
change the call in the loop to try the transformer-style signature first
(wrapper.predict(df, ticker_id=t_id, sector_id=s_id)) and on TypeError or
unexpected signature fallback to the simple positional call (wrapper.predict(df)
or wrapper.predict(X_input)) so all wrappers are compatible; additionally
normalize transformer return keys by detecting keys starting with "transformer"
(e.g., "transformer_1d","transformer_3d") from TransformerSignalModel and
aggregate or map them to a canonical "transformer" key (e.g., average the
transformer_* scores or pick the appropriate subkey) so later lookup that uses
signals.get("transformer", 0.5) finds the transformer signal.
Comment on lines
+46
to
+47
| # 💡 [수정 1] 개별 DB 함수들 대신 PortfolioRepository 클래스 하나만 임포트합니다. | ||
| from AI.libs.database.repository import PortfolioRepository |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# PortfolioRepository 클래스 정의 확인
rg -n "class PortfolioRepository" AI/libs/database/repository.pyRepository: SISC-IT/sisc-web
Length of output: 42
🏁 Script executed:
#!/bin/bash
# Check what actually exists in repository.py
echo "=== Contents of repository.py ==="
wc -l AI/libs/database/repository.py
head -80 AI/libs/database/repository.pyRepository: SISC-IT/sisc-web
Length of output: 2666
🏁 Script executed:
#!/bin/bash
# Search for PortfolioRepository definition anywhere in codebase
echo "=== Searching for PortfolioRepository class definition ==="
rg -n "class PortfolioRepository" --type pyRepository: SISC-IT/sisc-web
Length of output: 118
🏁 Script executed:
#!/bin/bash
# Check the actual usage in daily_routine.py around line 63
echo "=== Usage in daily_routine.py ==="
sed -n '40,80p' AI/pipelines/daily_routine.pyRepository: SISC-IT/sisc-web
Length of output: 1598
🏁 Script executed:
#!/bin/bash
# Check all imports in daily_routine.py
echo "=== All imports in daily_routine.py ==="
head -50 AI/pipelines/daily_routine.pyRepository: SISC-IT/sisc-web
Length of output: 1845
🏁 Script executed:
#!/bin/bash
# Get the full function names in repository.py
rg -n "^def " AI/libs/database/repository.pyRepository: SISC-IT/sisc-web
Length of output: 627
🏁 Script executed:
#!/bin/bash
# Check if PortfolioRepository is used elsewhere in the codebase
rg -n "PortfolioRepository" --type pyRepository: SISC-IT/sisc-web
Length of output: 324
🏁 Script executed:
#!/bin/bash
# Check what repo.method() calls exist in daily_routine.py to understand full impact
grep -n "repo\." AI/pipelines/daily_routine.py | head -20Repository: SISC-IT/sisc-web
Length of output: 611
PortfolioRepository 클래스가 존재하지 않음
repository.py에는 get_current_position(), get_current_cash(), save_executions_to_db() 등의 모듈 수준 함수만 정의되어 있고, PortfolioRepository 클래스는 존재하지 않습니다.
Line 63의 repo = PortfolioRepository(db_name="db") 및 이후 라인 222, 235, 337, 350, 369, 396, 408의 repo.method() 호출들이 모두 실패합니다.
수정안
-# 💡 [수정 1] 개별 DB 함수들 대신 PortfolioRepository 클래스 하나만 임포트합니다.
-from AI.libs.database.repository import PortfolioRepository
+from AI.libs.database.repository import (
+ get_current_position,
+ get_current_cash,
+ save_reports_to_db,
+ save_executions_to_db,
+ save_portfolio_summary,
+ save_portfolio_positions
+)그리고 모든 repo.method() 호출을 해당 함수의 직접 호출로 변경합니다 (예: repo.get_current_cash() → get_current_cash()).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@AI/pipelines/daily_routine.py` around lines 46 - 47, The code imports and
instantiates a non-existent class PortfolioRepository and then calls
repo.method() everywhere; instead revert to using the module-level functions
defined in AI.libs.database.repository (e.g., get_current_position,
get_current_cash, save_executions_to_db, etc.): remove the PortfolioRepository
import/instantiation (remove repo = PortfolioRepository(...)) and replace every
repo.get_current_cash(), repo.get_current_position(),
repo.save_executions_to_db(), etc. with direct calls to get_current_cash(),
get_current_position(), save_executions_to_db() (and import those functions from
AI.libs.database.repository) so all calls match the actual function names in the
module.
Comment on lines
+204
to
+206
| ticker_ids=loader.ticker_to_id, | ||
| ticker_to_sector_id=loader.ticker_to_sector_id, | ||
| gating_model=None, |
There was a problem hiding this comment.
AttributeError: loader.ticker_to_sector_id 속성이 존재하지 않음
DataLoader 클래스의 속성명은 ticker_sector_id이지만, 여기서는 ticker_to_sector_id로 접근하고 있습니다. 이 불일치로 인해 런타임에 AttributeError가 발생합니다.
🐛 제안 수정안
target_weights, scores, all_signals_map = calculate_portfolio_allocation(
data_map=data_map,
macro_data=dummy_macro_data,
model_wrappers=model_wrappers,
ticker_ids=loader.ticker_to_id,
- ticker_to_sector_id=loader.ticker_to_sector_id,
+ ticker_to_sector_id=loader.ticker_sector_id,
gating_model=None,
config=strategy_config
)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@AI/pipelines/daily_routine.py` around lines 204 - 206, The code references a
non-existent attribute loader.ticker_to_sector_id causing an AttributeError;
update the call site in daily_routine.py (the function passing
ticker_ids=loader.ticker_to_id, ticker_to_sector_id=...) to use the correct
DataLoader attribute loader.ticker_sector_id instead of
loader.ticker_to_sector_id, and scan for any other uses of ticker_to_sector_id
to replace them or adapt parameter names accordingly.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
schema.sql (1)
226-236:⚠️ Potential issue | 🟠 Major
portfolio_positions테이블은(date, ticker)조합의 유일성을 보장해야 합니다.테이블 설명에서 "일자별 종목 포지션 스냅샷"이며 "일별 상태 재현/검증"이 목적인데, 현재는 인덱스만 존재하고 제약이 없습니다. 인덱스는 조회 성능만 개선하며 중복을 막지 못하므로, 같은
(date, ticker)가 여러 번 들어오는 순간 재현 및 검증 결과가 손상됩니다.event_calendar와sector_returns테이블에서처럼 PRIMARY KEY 또는 UNIQUE 제약으로 변경하세요.제약으로 올리는 예시
CREATE TABLE "portfolio_positions" ( "date" date NOT NULL, -- 스냅샷 기준 날짜 "ticker" varchar(255) NOT NULL, -- 종목 티커 "position_qty" integer NOT NULL, -- 보유 수량 "avg_price" numeric(38, 6) NOT NULL, -- 평균 매입가 "current_price" numeric(38, 6) NOT NULL,-- 기준일 종가/평가 단가 "market_value" numeric(20, 6) NOT NULL, -- 평가금액 "pnl_unrealized" numeric(20, 6) NOT NULL,-- 미실현 손익 "pnl_realized_cum" numeric(20, 6) NOT NULL, -- 누적 실현 손익(해당 종목) - "created_at" timestamp with time zone DEFAULT now() NOT NULL -- 기록 시각 + "created_at" timestamp with time zone DEFAULT now() NOT NULL, -- 기록 시각 + CONSTRAINT "pk_portfolio_positions" PRIMARY KEY ("date", "ticker") ) TABLESPACE ts_ai_hdd; @@ -- 5. portfolio_positions: PK 누락 보완 및 날짜/종목 조회용 -CREATE INDEX IF NOT EXISTS "idx_portfolio_positions_date_ticker" ON "portfolio_positions" ("date", "ticker") TABLESPACE ts_ai_hdd;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@schema.sql` around lines 226 - 236, The portfolio_positions table must enforce uniqueness on the (date, ticker) pair to prevent duplicate daily snapshots: modify the table definition for portfolio_positions to add a PRIMARY KEY (date, ticker) or a UNIQUE (date, ticker) constraint (or ALTER TABLE to add the constraint if created already), and remove or leave any existing non-unique index that becomes redundant; update the CREATE TABLE for portfolio_positions to include PRIMARY KEY ("date","ticker") (or add a UNIQUE constraint) so the DB enforces one row per date+ticker.
🧹 Nitpick comments (1)
schema.sql (1)
125-140:idx_market_breadth_date는 PK 인덱스와 중복됩니다.Line 126의
PRIMARY KEY가 이미date접근 경로를 만들고 있으므로, 같은 컬럼에 보조 인덱스를 하나 더 두면 저장 공간과 쓰기 비용만 늘어납니다. 물리적 위치만 맞추려는 목적이면 중복 인덱스 대신 PK 정의 쪽을 조정하는 편이 맞습니다.중복 인덱스 제거 예시
-- 인덱스 -CREATE INDEX IF NOT EXISTS "idx_market_breadth_date" ON "market_breadth" ("date") TABLESPACE ts_ai_hdd;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@schema.sql` around lines 125 - 140, The table market_breadth already declares "date" as PRIMARY KEY, so remove the redundant secondary index creation (idx_market_breadth_date) to avoid duplicate index storage and write overhead; locate the CREATE INDEX IF NOT EXISTS "idx_market_breadth_date" statement and delete it (or drop it if applied in migrations), and if you intended to control index tablespace/physical placement ensure the PRIMARY KEY constraint on "date" is created with the desired tablespace instead of adding a duplicate index.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@schema.sql`:
- Around line 13-24: Add a migration that actually moves existing tables and
their indexes to the ts_ai_hdd tablespace instead of only declaring TABLESPACE
in schema.sql: create a new migration script that runs ALTER TABLE
"company_names" SET TABLESPACE ts_ai_hdd; ALTER TABLE "stock_info" SET
TABLESPACE ts_ai_hdd; and do the same for large tables referenced in schema.sql
such as "price_data" and "executions", plus corresponding ALTER INDEX ... SET
TABLESPACE commands for each index currently declared in the schema; ensure the
migration is executed during a planned maintenance window (or uses
pg_repack/online methods) to minimize locks and verify disk capacity before
running.
- Around line 6-7: The CREATE TABLESPACE ts_ai_hdd statement in schema.sql is a
cluster-level operation that won’t be run/available in our current deployment
and will break the 24 DDLs that reference ts_ai_hdd; either remove all
TABLESPACE references and let tables use the default tablespace, or move the
schema into a Flyway migration (rename schema.sql to
db/migration/V1__initial_schema.sql) but remove the CREATE TABLESPACE ts_ai_hdd
line so DBAs/infrastructure can provision the physical path separately; update
any DDLs that explicitly reference ts_ai_hdd to use the default tablespace if
you choose removal.
---
Outside diff comments:
In `@schema.sql`:
- Around line 226-236: The portfolio_positions table must enforce uniqueness on
the (date, ticker) pair to prevent duplicate daily snapshots: modify the table
definition for portfolio_positions to add a PRIMARY KEY (date, ticker) or a
UNIQUE (date, ticker) constraint (or ALTER TABLE to add the constraint if
created already), and remove or leave any existing non-unique index that becomes
redundant; update the CREATE TABLE for portfolio_positions to include PRIMARY
KEY ("date","ticker") (or add a UNIQUE constraint) so the DB enforces one row
per date+ticker.
---
Nitpick comments:
In `@schema.sql`:
- Around line 125-140: The table market_breadth already declares "date" as
PRIMARY KEY, so remove the redundant secondary index creation
(idx_market_breadth_date) to avoid duplicate index storage and write overhead;
locate the CREATE INDEX IF NOT EXISTS "idx_market_breadth_date" statement and
delete it (or drop it if applied in migrations), and if you intended to control
index tablespace/physical placement ensure the PRIMARY KEY constraint on "date"
is created with the desired tablespace instead of adding a duplicate index.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 148729cf-e709-4c9e-a688-cab6577f029a
📒 Files selected for processing (2)
AI/modules/finder/screener.pyschema.sql
Comment on lines
13
to
+24
| CREATE TABLE "company_names" ( | ||
| "company_name" varchar(100) PRIMARY KEY, -- 기업 한글/영문 정식 명칭 | ||
| "ticker" varchar(255) NOT NULL UNIQUE -- 종목 티커 | ||
| ); | ||
| ) TABLESPACE ts_ai_hdd; | ||
|
|
||
| CREATE TABLE "stock_info" ( | ||
| "ticker" varchar(20) PRIMARY KEY, -- 종목 티커 | ||
| "sector" varchar(100), -- 섹터 분류 (예: IT, 금융) | ||
| "industry" varchar(200), -- 세부 산업 분류 | ||
| "market_cap" bigint, -- 시가총액 (필터링/비중 계산용) | ||
| "updated_at" timestamp DEFAULT now() -- 정보 갱신 일시 | ||
| ); | ||
| ) TABLESPACE ts_ai_hdd; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C2 'ALTER\s+TABLE.+SET\s+TABLESPACE|ALTER\s+INDEX.+SET\s+TABLESPACE|REINDEX.+TABLESPACE' .Repository: SISC-IT/sisc-web
Length of output: 42
🏁 Script executed:
find . -type f -name "*migration*" -o -name "*migrate*" | head -20Repository: SISC-IT/sisc-web
Length of output: 42
🏁 Script executed:
git ls-files | grep -E "(migration|migrate|sql)" | head -20Repository: SISC-IT/sisc-web
Length of output: 70
🏁 Script executed:
wc -l schema.sqlRepository: SISC-IT/sisc-web
Length of output: 74
🏁 Script executed:
# Check if price_data and executions tables are in schema.sql
rg -n "CREATE TABLE.*price_data|CREATE TABLE.*executions" .Repository: SISC-IT/sisc-web
Length of output: 199
🏁 Script executed:
sed -n '275,295p' schema.sqlRepository: SISC-IT/sisc-web
Length of output: 1281
🏁 Script executed:
# Check what TABLESPACE price_data and executions use currently
sed -n '31,50p' schema.sqlRepository: SISC-IT/sisc-web
Length of output: 993
🏁 Script executed:
sed -n '181,200p' schema.sqlRepository: SISC-IT/sisc-web
Length of output: 1152
🏁 Script executed:
# Also check what TABLESPACE is being set elsewhere in the file
rg -n "TABLESPACE" schema.sqlRepository: SISC-IT/sisc-web
Length of output: 1554
기존 테이블 마이그레이션용 ALTER TABLE/INDEX SET TABLESPACE 문이 없습니다.
확인 결과, 저장소에 ALTER TABLE ... SET TABLESPACE 또는 ALTER INDEX ... SET TABLESPACE를 수행하는 마이그레이션 파일이 존재하지 않습니다. 현재 schema.sql의 모든 테이블(price_data 라인 31, executions 라인 181 등)과 인덱스(라인 275-295)는 생성 시점에 TABLESPACE ts_ai_hdd로 정의되어 있으나, 이미 운영 DB에 존재하는 테이블은 이 정의만으로 실제로 이동되지 않습니다.
데이터 마이그레이션이 필요하다면 별도의 마이그레이션 스크립트를 추가해야 하며, 특히 대용량 테이블의 경우 잠금 시간과 저장소 용량을 고려하여 유지보수 시간대에 실행해야 합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@schema.sql` around lines 13 - 24, Add a migration that actually moves
existing tables and their indexes to the ts_ai_hdd tablespace instead of only
declaring TABLESPACE in schema.sql: create a new migration script that runs
ALTER TABLE "company_names" SET TABLESPACE ts_ai_hdd; ALTER TABLE "stock_info"
SET TABLESPACE ts_ai_hdd; and do the same for large tables referenced in
schema.sql such as "price_data" and "executions", plus corresponding ALTER INDEX
... SET TABLESPACE commands for each index currently declared in the schema;
ensure the migration is executed during a planned maintenance window (or uses
pg_repack/online methods) to minimize locks and verify disk capacity before
running.
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
릴리스 노트
새로운 기능
개선 사항