Feat(#36): 형태소·문자 기반 피싱 분류 모델 추가 및 성능 비교 (1/2) - #38
Conversation
📝 WalkthroughWalkthroughThis PR adds shared SMS preprocessing, Kiwi tokenization, template grouping, leakage-safe grouped splits, Naive Bayes evaluation, reporting, artifact export, and updated model paths. ChangesSMS model pipeline
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Trivy (0.72.0)Trivy execution failed: 2026-08-07T07:55:01Z FATAL Fatal error run error: fs scan error: scan error: scan failed: failed analysis: post analysis error: post analysis error: helm scan error: fs filter error: fs filter error: walk error range error: stat .coderabbit-opengrep-fallback.8e2aa0dd-523f-46e8-9424-76d99d76c095.yml: no such file or directory: range error: stat .coderabbit-opengrep-fallback.8e2aa0dd-523f-46e8-9424-76d99d76c095.yml: no such file or directory 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: 12
🧹 Nitpick comments (2)
data_science/SMSModel/train_sms.py (2)
235-324: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winConsider failing when the committed manifest is absent.
The README states that the committed manifest fixes the final test set. When
SPLIT_MANIFEST_PATHdoes not exist andcreate_manifestisFalse,split_datagenerates and saves a new manifest anyway.run_naive_bayes_baseline.pycallssplit_data(dataset)with the same defaults, so baseline metrics can be produced against an ad-hoc test set. The print at Line 274 is the only signal. Requirecreate_manifest=Trueto create a manifest, so an accidental regeneration stops the run.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@data_science/SMSModel/train_sms.py` around lines 235 - 324, Update split_data so that when SPLIT_MANIFEST_PATH is absent and create_manifest is False, it raises an error instead of generating or saving a new split; only the explicit create_manifest=True path may call split_grouped_dataset and save_split_manifest, while the existing manifest-loading path remains unchanged.
141-142: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winThe null check covers columns that the schema does not require.
required_columnslists onlytext,label,type, andhas_url.df.isnull().any().any()rejects nulls in every column, including the optionalsourcecolumn that Line 160 reads with a default. A dataset that adds any optional column with blank cells fails to load, and the error message does not name the column. Restrict the check to the required columns and report the offending ones.♻️ Proposed change
- if df.isnull().any().any(): - raise ValueError("결측치가 존재합니다.") + null_columns = [ + column + for column in required_columns + if df[column].isnull().any() + ] + if null_columns: + raise ValueError(f"결측치가 존재하는 컬럼: {sorted(null_columns)}")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@data_science/SMSModel/train_sms.py` around lines 141 - 142, Update the null validation in the training-data loading flow to inspect only the columns listed by required_columns (text, label, type, and has_url), leaving optional columns such as source eligible for the existing default handling. Collect the required columns containing nulls and include their names in the ValueError message instead of reporting a generic failure.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/analysis/text/naive_bayes_analyzer.py`:
- Around line 103-110: Update the result construction in the analyzer method so
result["error_message"] always uses DEFAULT_ANALYSIS_RESULT["error_message"]
rather than _load_error; retain _load_error only for internal logging where the
exception type belongs.
- Around line 58-61: Update _load_artifacts to synchronize concurrent callers
with a lock: acquire the lock before checking _load_attempted, re-check the flag
after acquiring it, and keep the flag unset until the artifact load completes or
otherwise ensure waiting callers do not return while _model is still
unavailable. Preserve the existing one-time loading behavior and have concurrent
startup requests observe the successfully loaded model.
- Around line 116-119: The single-message feature construction in
naive_bayes_analyzer.py and predict_risk_score does not reproduce the declared
has_url training signal. Update each extract_struct_features call to pass the
same has_url value used by extract_struct_feature_matrix, including the site at
data_science/SMSModel/train_sms.py:546, or consistently derive that value
identically during training and serving.
In `@data_science/SMSModel/dataset_splitting/manifest.py`:
- Around line 14-20: Update the manifest-building and saving flow to accept and
use DatasetSplitConfig, including build_split_manifest and save_split_manifest,
instead of relying on fixed MANIFEST_COLUMNS names. Derive the fingerprint,
group, and label columns from the configuration so renamed key columns work end
to end, and add a round-trip test covering custom column names.
In `@data_science/SMSModel/dataset_splitting/splitter.py`:
- Around line 85-92: Update _select_best_group_split and its
candidate-generation flow so candidates vary across feasible group counts
instead of fixing one count before scoring. Ensure _candidate_score evaluates
each whole-group split against the configured train, validation, and test row
ratios (while preserving label-distribution scoring), allowing unequal group
sizes to be optimized for row-level targets.
In `@data_science/SMSModel/evaluation/threshold.py`:
- Around line 53-58: Update the validation logic in the threshold-selection
function around allowed_labels to reject y_true when it contains only one
distinct supported label, raising ValueError before computing or returning any
score-derived threshold. Preserve the existing unsupported-label validation, and
add coverage for normal-only and phishing-only validation data.
In `@data_science/SMSModel/modeling/artifacts.py`:
- Around line 51-65: The save_operational_naive_bayes_artifacts() flow currently
exposes model_path and vectorizer_path independently, allowing mixed artifact
generations during reloads. Store both outputs in a new versioned directory,
then atomically update a single manifest or pointer only after both files are
successfully written; update the API loader to resolve both artifacts through
that shared pointer.
In `@data_science/SMSModel/modeling/base.py`:
- Around line 28-37: Update __post_init__ to assign the normalized result of
np.asarray(self.values) back to self.values before validation, so list inputs
are stored as NumPy arrays. Preserve the existing dimensionality, finiteness,
and probability-range checks.
In `@data_science/SMSModel/modeling/naive_bayes.py`:
- Around line 210-228: Before constructing or fitting CalibratedClassifierCV in
the training flow, use train_df["label"].value_counts().min() to validate that
every class has at least self.calibration_cv samples. Raise the established
validation error when the minimum count is insufficient, while preserving
_validate_dataframe and the existing calibration setup for valid datasets.
In `@data_science/SMSModel/README.md`:
- Around line 3-13: Add tokenization/, modeling/, evaluation/, and
run_naive_bayes_baseline.py as rows in the workspace table, with concise
purposes matching their roles, so the README inventory includes every newly
added package and baseline runner.
In `@Dockerfile`:
- Around line 13-19: Ensure the runtime image includes the module path required
by the serialized vectorizer’s kiwi_tokenize callable. Update the Dockerfile
COPY steps to include data_science/SMSModel/tokenization/__init__.py and
kiwi_tokenizer.py, or relocate kiwi_tokenize under an already-copied package
while preserving its import path at model load time.
In `@tests/data_science/SMSModel/test_template_grouping.py`:
- Around line 305-307: Rename the unused holdout result binding in the load_data
call to _df_holdout, while preserving df_pool and the existing load_data
invocation.
---
Nitpick comments:
In `@data_science/SMSModel/train_sms.py`:
- Around line 235-324: Update split_data so that when SPLIT_MANIFEST_PATH is
absent and create_manifest is False, it raises an error instead of generating or
saving a new split; only the explicit create_manifest=True path may call
split_grouped_dataset and save_split_manifest, while the existing
manifest-loading path remains unchanged.
- Around line 141-142: Update the null validation in the training-data loading
flow to inspect only the columns listed by required_columns (text, label, type,
and has_url), leaving optional columns such as source eligible for the existing
default handling. Collect the required columns containing nulls and include
their names in the ValueError message instead of reporting a generic failure.
🪄 Autofix
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 Plus
Run ID: 870ca283-5df5-497c-8d82-5dfa1d9579aa
⛔ Files ignored due to path filters (8)
data_science/SMSModel/artifacts/phishing_model_artifact.pklis excluded by!**/*.pkldata_science/SMSModel/artifacts/phishing_vectorizer.pklis excluded by!**/*.pkldata_science/SMSModel/reports/feature_scores_full.csvis excluded by!**/*.csvdata_science/SMSModel/reports/figures/feature_importance.pngis excluded by!**/*.pngdata_science/SMSModel/reports/figures/risk_distribution_fig1.pngis excluded by!**/*.pngdata_science/SMSModel/reports/figures/risk_distribution_fig2.pngis excluded by!**/*.pngdata_science/SMSModel/reports/model_evaluation/naive_bayes_baseline/model_evaluation.csvis excluded by!**/*.csvdata_science/SMSModel/splits/sms_split_v1.csvis excluded by!**/*.csv
📒 Files selected for processing (62)
.dockerignore.env.exampleDockerfileSCORING_PIPELINE_CHANGES.mdapp/analysis/text/naive_bayes_analyzer.pyapp/analysis/text/preprocessing.pyapp/core/config.pydata_science/SMSModel/README.mddata_science/SMSModel/SMSDataModel.ipynbdata_science/SMSModel/dataset_splitting/__init__.pydata_science/SMSModel/dataset_splitting/config.pydata_science/SMSModel/dataset_splitting/manifest.pydata_science/SMSModel/dataset_splitting/splitter.pydata_science/SMSModel/dataset_splitting/validation.pydata_science/SMSModel/evaluation/__init__.pydata_science/SMSModel/evaluation/evaluator.pydata_science/SMSModel/evaluation/latency.pydata_science/SMSModel/evaluation/metrics.pydata_science/SMSModel/evaluation/reporting.pydata_science/SMSModel/evaluation/threshold.pydata_science/SMSModel/modeling/__init__.pydata_science/SMSModel/modeling/artifacts.pydata_science/SMSModel/modeling/base.pydata_science/SMSModel/modeling/naive_bayes.pydata_science/SMSModel/reporting/__init__.pydata_science/SMSModel/reporting/dataset_split_report.pydata_science/SMSModel/reports/dataset_split_summary.jsondata_science/SMSModel/reports/dataset_split_summary.mddata_science/SMSModel/reports/model_evaluation/naive_bayes_baseline/model_evaluation.jsondata_science/SMSModel/reports/model_evaluation/naive_bayes_baseline/model_evaluation.mddata_science/SMSModel/run_naive_bayes_baseline.pydata_science/SMSModel/template_grouping/__init__.pydata_science/SMSModel/template_grouping/config.pydata_science/SMSModel/template_grouping/fingerprint.pydata_science/SMSModel/template_grouping/service.pydata_science/SMSModel/template_grouping/similarity.pydata_science/SMSModel/tokenization/__init__.pydata_science/SMSModel/tokenization/kiwi_tokenizer.pydata_science/SMSModel/train_sms.pypytest.inirequirements.txttests/analysis/text/test_naive_bayes_analyzer.pytests/analysis/text/test_preprocessing.pytests/data_science/SMSModel/__init__.pytests/data_science/SMSModel/evaluation/__init__.pytests/data_science/SMSModel/evaluation/conftest.pytests/data_science/SMSModel/evaluation/test_base.pytests/data_science/SMSModel/evaluation/test_evaluator.pytests/data_science/SMSModel/evaluation/test_latency.pytests/data_science/SMSModel/evaluation/test_metrics.pytests/data_science/SMSModel/evaluation/test_reporting.pytests/data_science/SMSModel/evaluation/test_threshold.pytests/data_science/SMSModel/modeling/conftest.pytests/data_science/SMSModel/modeling/test_artifacts.pytests/data_science/SMSModel/modeling/test_baseline_runner.pytests/data_science/SMSModel/modeling/test_naive_bayes.pytests/data_science/SMSModel/test_dataset_split_report.pytests/data_science/SMSModel/test_dataset_splitting.pytests/data_science/SMSModel/test_template_grouping.pytests/data_science/SMSModel/tokenization/__init__.pytests/data_science/SMSModel/tokenization/test_kiwi_tokenizer.pytests/data_science/__init__.py
| if _load_attempted: | ||
| return | ||
|
|
||
| _load_attempted = True |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Concurrent first requests can receive a spurious unavailable result.
_load_artifacts sets _load_attempted = True before the load starts. A second request that arrives while joblib.load is still running returns at Line 59 with _model still None. analyze_text_with_naive_bayes then reports is_available: False even though the artifact loads correctly. FastAPI runs sync work in a thread pool, so this window is reachable under concurrent traffic at startup.
Guard the load with a lock and re-check the flag inside it.
🔒 Proposed fix
import logging
+import threading
import numpy as np+_load_lock = threading.Lock()
_load_attempted = False- if _load_attempted:
- return
-
- _load_attempted = True
-
- try:
- import joblib
-
- artifact = joblib.load(MODEL_PATH)
- vectorizer = joblib.load(VECTORIZER_PATH)
-
- # 모든 값이 정상적으로 읽힌 이후 전역 상태를 갱신
- _model = artifact["model"]
- _threshold = artifact["threshold"]
- _classes = artifact["classes"]
- _vectorizer = vectorizer
- _load_error = None
-
- logger.info(
- "[NaiveBayes] 모델 로드 완료 (threshold=%s)",
- _threshold,
- )
- except Exception as exception:
- _load_error = type(exception).__name__
-
- logger.error(
- "[NaiveBayes] 모델 로드 실패. error_type=%s",
- _load_error,
- )
+ if _load_attempted:
+ return
+
+ with _load_lock:
+ if _load_attempted:
+ return
+
+ try:
+ import joblib
+
+ artifact = joblib.load(MODEL_PATH)
+ vectorizer = joblib.load(VECTORIZER_PATH)
+
+ # 모든 값이 정상적으로 읽힌 이후 전역 상태를 갱신
+ _model = artifact["model"]
+ _threshold = artifact["threshold"]
+ _classes = artifact["classes"]
+ _vectorizer = vectorizer
+ _load_error = None
+
+ logger.info(
+ "[NaiveBayes] 모델 로드 완료 (threshold=%s)",
+ _threshold,
+ )
+ except Exception as exception:
+ _load_error = type(exception).__name__
+
+ logger.error(
+ "[NaiveBayes] 모델 로드 실패. error_type=%s",
+ _load_error,
+ )
+ finally:
+ _load_attempted = True📝 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.
| if _load_attempted: | |
| return | |
| _load_attempted = True | |
| if _load_attempted: | |
| return | |
| with _load_lock: | |
| if _load_attempted: | |
| return | |
| try: | |
| import joblib | |
| artifact = joblib.load(MODEL_PATH) | |
| vectorizer = joblib.load(VECTORIZER_PATH) | |
| # 모든 값이 정상적으로 읽힌 이후 전역 상태를 갱신 | |
| _model = artifact["model"] | |
| _threshold = artifact["threshold"] | |
| _classes = artifact["classes"] | |
| _vectorizer = vectorizer | |
| _load_error = None | |
| logger.info( | |
| "[NaiveBayes] 모델 로드 완료 (threshold=%s)", | |
| _threshold, | |
| ) | |
| except Exception as exception: | |
| _load_error = type(exception).__name__ | |
| logger.error( | |
| "[NaiveBayes] 모델 로드 실패. error_type=%s", | |
| _load_error, | |
| ) | |
| finally: | |
| _load_attempted = True |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/analysis/text/naive_bayes_analyzer.py` around lines 58 - 61, Update
_load_artifacts to synchronize concurrent callers with a lock: acquire the lock
before checking _load_attempted, re-check the flag after acquiring it, and keep
the flag unset until the artifact load completes or otherwise ensure waiting
callers do not return while _model is still unavailable. Preserve the existing
one-time loading behavior and have concurrent startup requests observe the
successfully loaded model.
| "result": dict( | ||
| DEFAULT_ANALYSIS_RESULT, | ||
| error_message=( | ||
| _load_error | ||
| or DEFAULT_ANALYSIS_RESULT["error_message"] | ||
| ), | ||
| ), | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Do not return the exception class name as the user-facing error message.
_load_error holds type(exception).__name__, for example FileNotFoundError or UnpicklingError. This value is placed directly in result["error_message"], which the API returns to clients. The value exposes internal failure detail and breaks the localized message contract used everywhere else in DEFAULT_ANALYSIS_RESULT. Keep the exception type in the log only.
🛡️ Proposed fix
"result": dict(
DEFAULT_ANALYSIS_RESULT,
- error_message=(
- _load_error
- or DEFAULT_ANALYSIS_RESULT["error_message"]
- ),
+ error_message=DEFAULT_ANALYSIS_RESULT["error_message"],
),📝 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.
| "result": dict( | |
| DEFAULT_ANALYSIS_RESULT, | |
| error_message=( | |
| _load_error | |
| or DEFAULT_ANALYSIS_RESULT["error_message"] | |
| ), | |
| ), | |
| } | |
| "result": dict( | |
| DEFAULT_ANALYSIS_RESULT, | |
| error_message=DEFAULT_ANALYSIS_RESULT["error_message"], | |
| ), | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/analysis/text/naive_bayes_analyzer.py` around lines 103 - 110, Update the
result construction in the analyzer method so result["error_message"] always
uses DEFAULT_ANALYSIS_RESULT["error_message"] rather than _load_error; retain
_load_error only for internal logging where the exception type belongs.
| struct_features = np.asarray( | ||
| [extract_struct_features(text)], | ||
| dtype=np.int8, | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Compare the dataset has_url column against the shared URL regex.
fd -t f 'phishing_total_dataset_2705.csv' | head -n 1 | while IFS= read -r csv; do
python - "$csv" <<'PY'
import sys, pandas as pd
sys.path.insert(0, ".")
from app.analysis.text.preprocessing import URL_PATTERN
df = pd.read_csv(sys.argv[1])
detected = df["text"].astype(str).str.contains(URL_PATTERN)
declared = df["has_url"].astype(bool)
print("rows:", len(df), "mismatches:", int((detected != declared).sum()))
print(df.loc[detected != declared, ["text", "has_url"]].head(10).to_string())
PY
doneRepository: SafeFam/SafeFam_AI
Length of output: 275
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== files ==="
fd -t f 'naive_bayes_analyzer.py|train_sms.py|naive_bayes.py|preprocessing.py|phishing_total_dataset_2705.csv' .
echo
echo "=== relevant snippets ==="
for f in app/analysis/text/naive_bayes_analyzer.py data_science/SMSModel/train_sms.py data_science/SMSModel/modeling/naive_bayes.py app/analysis/text/preprocessing.py; do
if [ -f "$f" ]; then
echo "--- $f ---"
ast-grep outline "$f" --match 'extract_struct_features|extract_struct_feature_matrix|_build_feature_matrix|URL_PATTERN' --view expanded || true
rg -n -C 4 'def extract_struct_features|def extract_struct_feature_matrix|def _build_feature_matrix|has_url|URL_PATTERN' "$f"
fi
done
echo
echo "=== dataset has_url and text-based count (no pandas) ==="
csv="$(fd -t f 'phishing_total_dataset_2705.csv' . | head -n 1 || true)"
if [ -n "${csv:-}" ]; then
python3 - "$csv" <<'PY'
import sys, re
csv=sys.argv[1]
with open(csv, encoding='utf-8') as f:
header=f.readline().strip().split(',')
has_text=header.index('text')
has_url=header.index('has_url')
rows=list(csv.strip().splitlines())
print("csv:", csv, "rows:", len(rows)-1)
# read exact columns without pandas
PY
url_pattern="$(python3 -c "import ast, pathlib, re; f=pathlib.Path('app/analysis/text/preprocessing.py'); tree=ast.parse(f.read_text()); constants={n.value.id:'SUFFIX_PATTERN' in n.value.value if isinstance(n.value, ast.BinOp) else n.value.value for n in ast.walk(tree) if isinstance(n, ast.Assign)}; print(repr(__import__('glob').glob('SUFFIX_PATTERN')[0] if False else ''))")"
python3 - "$csv" <<'PY'
import sys, re, ast, pathlib
csv=sys.argv[1]
mod=pathlib.Path('app/analysis/text/preprocessing.py')
source=mod.read_text()
tree=ast.parse(source)
for node in ast.walk(tree):
if isinstance(node, ast.Assign):
for target in node.targets:
if isinstance(target, ast.Name) and target.id == 'URL_PATTERN':
print("URL_PATTERN=", ast.literal_eval(node.value.value))
PY
python3 - <<'PY' "$csv"
import sys, re, ast, pathlib
csv=sys.argv[1]
mod=pathlib.Path('app/analysis/text/preprocessing.py')
source=mod.read_text()
tree=ast.parse(source)
url_re=None
for node in ast.walk(tree):
if isinstance(node, ast.Assign):
for target in node.targets:
if isinstance(target, ast.Name) and target.id == 'URL_PATTERN':
url_re=ast.literal_eval(node.value.value)
p=re.compile(url_re)
with open(csv, encoding='utf-8') as f:
header=f.readline().strip().split(',')
has_text=header.index('text')
has_url=header.index('has_url')
rows=list(csv.strip().splitlines())
print("rows:", len(rows)-1, "url_re:", url_re)
mism=0
examples=[]
for i,row in enumerate(rows, start=2):
fields=row.split(',', header.index('has_url')+1)
txt=fields[has_text]
declared=bool(int(fields[has_url])) if fields[has_url] else False
detected=bool(p.search(txt))
if declared != detected:
mism += 1
if len(examples) < 10:
examples.append((i, txt[:200], declared, detected))
print("mismatches:", mism)
for rec in examples:
print(rec)
PY
else
echo "no dataset found"
fiRepository: SafeFam/SafeFam_AI
Length of output: 683
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== source snippets ==="
for f in app/analysis/text/naive_bayes_analyzer.py data_science/SMSModel/train_sms.py data_science/SMSModel/modeling/naive_bayes.py app/analysis/text/preprocessing.py; do
if [ -f "$f" ]; then
echo "--- $f ---"
wc -l "$f"
rg -n -C 4 'extract_struct_features|extract_struct_feature_matrix|build_feature_matrix|_build_feature_matrix|has_url|URL_PATTERN' "$f" || true
fi
done
echo
echo "=== URL_PATTERN ==="
cd app/analysis/text
python3 - <<'PY'
import ast
from pathlib import Path
source = Path('preprocessing.py').read_text()
tree = ast.parse(source)
for node in ast.walk(tree):
if isinstance(node, ast.Assign):
for target in node.targets:
if isinstance(target, ast.Name) and target.id == 'URL_PATTERN':
print(ast.literal_eval(node.value.value))
PY
echo
echo "=== dataset count without pandas ==="
csv="$(fd -t f 'phishing_total_dataset_2705.csv' data_science/Data/SMSData | head -n 1 || true)"
if [ -n "${csv:-}" ]; then
python3 - "$csv" <<'PY'
import re, ast, sys, pathlib
csv = sys.argv[1]
source = pathlib.Path('app/analysis/text/preprocessing.py').read_text()
tree = ast.parse(source)
url_re = None
for node in ast.walk(tree):
if isinstance(node, ast.Assign):
for target in node.targets:
if isinstance(target, ast.Name) and target.id == 'URL_PATTERN':
url_re = ast.literal_eval(node.value.value)
print("url_re:", url_re)
p = re.compile(url_re)
url_index = None
with open(csv, encoding='utf-8') as f:
header = f.readline().strip().split(',')
for i, h in enumerate(header):
if h.startswith('contains_url') or h == 'has_url' or h == 'has_url?' or h == 'has_url_':
url_index = i
break
if url_index is None:
print("no contains_url/has_url column")
sys.exit(0)
text_index = header.index('text') if 'text' in header else -1
rows = []
for line in f:
fields = line.strip().split(',', header.index('contains_url?') + 1)
text = fields[text_index]
rows.append((bool(p.search(text)), bool(int(fields[url_index]))))
same = sum(a == b for a, b in rows)
print("rows:", len(rows), "same:", same, "mismatches:", len(rows) - same)
mismatches = [(a,b) for a,b in rows if a != b]
if mismatches:
print("mismatch_fraction:", len(mismatches)/len(rows))
print("first_mismatches:", mismatches[:10])
PY
fiRepository: SafeFam/SafeFam_AI
Length of output: 8073
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import re, sys, ast, pathlib
csv = sys.argv[1]
url_re = r"(?i)(?<!@)(?:https?://|www\.)[A-Za-z0-9\-._~:/?#\[\]@!$&'()*+;=%]+"
p = re.compile(url_re)
try:
idx = next(i for i, h in enumerate(open(csv, encoding='utf-8').readline().strip().split(',')) if h == 'has_url')
except StopIteration:
print("missing has_url")
sys.exit(0)
with open(csv, encoding='utf-8') as f:
header = f.readline().strip().split(',')
text_idx = header.index('text')
row_count = 0
same = 0
examples = []
for line in f:
row_count += 1
fields = line.rstrip('\n').split(',', idx + 1)
detected = bool(p.search(fields[text_idx]))
declared = bool(int(fields[idx])) if fields[idx] else False
if detected != declared:
if len(examples) < 10:
examples.append((row_count, decoded(fields[text_idx])[1][100:], declared, detected))
else:
same += 1
def decoded(s):
try: return bytes.fromhex(s).decode('utf-8', 'replace')
except Exception: return s
print("rows:", row_count, "same:", same, "mismatches:", row_count - same)
for e in examples:
print(e)
PY
data_science/Data/SMSData/phishing_total_dataset_2705.csvRepository: SafeFam/SafeFam_AI
Length of output: 265
🏁 Script executed:
#!/bin/bash
set -euo pipefail
csv="$(fd -t f 'phishing_total_dataset_2705.csv' . | head -n 1 || true)"
echo "=== dataset header ==="
if [ -n "${csv:-}" ]; then
python3 - "$csv" <<'PY'
import sys
path = sys.argv[1]
with open(path, encoding='utf-8') as f:
line = f.readline()
print(line.strip())
PY
echo "url-pattern match counts"
python3 - "$csv" <<'PY'
import re, sys
p = re.compile(r"(?i)(?<!@)(?:https?://|www\.)[A-Za-z0-9\-._~:/?#\[\]@!$&'()*+;=%]+")
path = sys.argv[1]
with open(path, encoding='utf-8') as f:
header = f.readline().strip().split(',')
print("header_count=", len(header), "contains_url_position=", next((i,header[i]) for i,h in enumerate(header) if h=="contains_url?"), sep="#")
text_i=header.index('text')
contains_url_i=header.index('contains_url?')
total=0; ppos=0; npos=0; mism=0
for i,line in enumerate(f,2):
total += 1
fields=line.rstrip('\n').split(',')
text=fields[text_i]
declared=bool(int(fields[contains_url_i])) if fields[contains_url_i] else False
detected=bool(p.search(text))
if declared:
ppos +=1
else:
npos +=1
if declared != detected:
mism +=1
if mism <= 5:
print(f"row {i}: txt_len={len(text)} declared={declared} detected={detected} txt={text[:120]}")
print("total_rows", total, "pos_declared", ppos, "neg_declared", npos, "same", total-mism, "mismatches", mism)
PY
fiRepository: SafeFam/SafeFam_AI
Length of output: 323
🏁 Script executed:
#!/bin/bash
set -euo pipefail
csv="$(fd -t f 'phishing_total_dataset_2705.csv' . | head -n 1 || true)"
echo "=== dataset header =="
if [ -n "${csv:-}" ]; then
python3 - "$csv" <<'PY'
import sys
path = sys.argv[1]
with open(path, encoding='utf-8') as f:
line = f.readline()
print(line.strip())
print("has_url position:", next((i, h) for i, h in enumerate(line.strip().split(',')) if h == 'has_url'))
PY
echo "=== url-pattern vs has_url =="
python3 - "$csv" <<'PY'
import re, sys, ast, pathlib
path = sys.argv[1]
url_re = r"(?i)(?<!@)(?:https?://|www\.)[A-Za-z0-9\-._~:/?#\[\]@!$&'()*+;=%]+"
p = re.compile(url_re)
with open(path, encoding='utf-8') as f:
header = f.readline().strip().split(',')
text_i = header.index('text')
has_url_i = header.index('has_url')
total = same = mism = 0
examples = []
for i, line in enumerate(f, 2):
total += 1
fields = line.rstrip('\n').split(',')
txt = fields[text_i]
declared = bool(int(fields[has_url_i])) if fields[has_url_i] else False
detected = bool(p.search(txt))
if declared == detected:
same += 1
else:
mism += 1
if len(examples) < 10:
examples.append((i, len(txt), declared, detected, txt[:80]))
print("rows", total)
print("same", same)
print("mismatches", mism)
print("mismatches_fraction", mism / total)
for ex in examples:
print(ex)
PY
fiRepository: SafeFam/SafeFam_AI
Length of output: 380
🏁 Script executed:
#!/bin/bash
set -euo pipefail
csv="$(fd -t f 'phishing_total_dataset_2705.csv' . | head -n 1 || true)"
echo "=== column indices for URL_PATTERN regex used in preprocessing.py =="
python3 - "$csv" <<'PY'
import re, sys
path = sys.argv[1]
url_re = r"(?i)(?<!@)(?:https?://|www\.)[A-Za-z0-9\-._~:/?#\[\]@!$&'()*+;=%]+"
p = re.compile(url_re)
with open(path, encoding='utf-8') as f:
header = f.readline().strip().split(',')
print(header)
text_i = next(i for i, h in enumerate(header) if h == 'text')
has_url_i = next(i for i, h in enumerate(header) if h == 'has_url')
text_escaped_re = re.escape(header[text_i])
has_url_escaped_re = re.escape(header[has_url_i])
print(text_i)
print(has_url_i)
match = p.search(header[has_url_i])
print(match) if match else print("URL_PATTERN matches header's has_url column")
PY
echo "=== raw head =="
python3 - "$csv" <<'PY'
import sys
path = sys.argv[1]
with open(path, encoding='utf-8', newline='') as f:
for i in range(5):
print(f"line[{i}] repr={next(f).strip()!r}")
PY
echo "=== URL_FEATURE from contains_url-like fields if any =="
python3 - "$csv" <<'PY'
import re, sys
path = sys.argv[1]
with open(path, encoding='utf-8') as f:
header = f.readline().strip().split(',')
text_i = next(i for i, h in enumerate(header) if h == 'text')
for i,h in enumerate(header):
if h.startswith('contains_url') or h in ('has_url', 'has_url_') or h == 'has_url?':
print(f"column {i}: {h}")
PYRepository: SafeFam/SafeFam_AI
Length of output: 365
🏁 Script executed:
#!/bin/bash
set -euo pipefail
csv="$(fd -t f 'phishing_total_dataset_2705.csv' . | head -n 1 || true)"
echo "=== raw dataset head =="
python3 - "$csv" <<'PY'
import sys
path = sys.argv[1]
with open(path, encoding='utf-8-sig') as f:
for i in range(6):
line = next(f, None)
print(f"line[{i}] len={len(line)} text={next(f, '')!r}")
PY
echo "=== exact mismatch report against column names =="
python3 - "$csv" <<'PY'
import re, sys, glob
path = sys.argv[1]
url_re = r"(?i)(?<!@)(?:https?://|www\.)[A-Za-z0-9\-._~:/?#\[\]@!$&'()*+;=%]+"
p = re.compile(url_re)
with open(path, encoding='utf-8-sig') as f:
header = f.readline().strip().split(',')
print("header=[{}]" .format(','.join(repr(h) for h in header)))
text_i = next(i for i, h in enumerate(header) if h == 'text')
has_url_i = next(i for i, h in enumerate(header) if h == 'has_url')
total = same = mism = 0
examples = []
for i in range(2, 6)
line = next(f, None)
print(i, line.strip()[:100], text_i, has_url_i)
PYRepository: SafeFam/SafeFam_AI
Length of output: 1289
🏁 Script executed:
#!/bin/bash
set -euo pipefail
csv="$(fd -t f 'phishing_total_dataset_2705.csv' . | head -n 1 || true)"
echo "=== exact mismatch report =="
python3 - "$csv" <<'PY'
import re, sys
path = sys.argv[1]
url_re = r"(?i)(?<!@)(?:https?://|www\.)[A-Za-z0-9\-._~:/?#\[\]@!$&'()*+;=%]+"
p = re.compile(url_re)
with open(path, encoding='utf-8-sig') as f:
header = f.readline().strip().split(',')
text_i = header.index('text')
has_url_i = header.index('has_url')
total = same = mism = 0
examples = []
for row_no, row in enumerate(f, start=2):
fields = row.rstrip('\n').split(',', has_url_i + 1)
txt = fields[text_i]
declared = bool(int(fields[has_url_i])) if fields[has_url_i] else False
detected = bool(p.search(txt))
if declared == detected:
same += 1
else:
mism += 1
if len(examples) < 10:
examples.append((row_no, len(txt), declared, detected, txt[:120]))
print("rows", total)
print("same", same)
print("mismatches", mism)
print("mismatches_fraction", mism / total if total else 0)
for ex in examples:
print(ex)
PYRepository: SafeFam/SafeFam_AI
Length of output: 301
🏁 Script executed:
#!/bin/bash
set -euo pipefail
csv="$(fd -t f 'phishing_total_dataset_2705.csv' . | head -n 1 || true)"
python3 - "$csv" <<'PY'
import re, sys, pathlib
path = sys.argv[1]
url_re = r"(?i)(?<!@)(?:https?://|www\.)[A-Za-z0-9\-._~:/?#\[\]@!$&'()*+;=%]+"
p = re.compile(url_re)
with open(path, encoding='utf-8-sig') as f:
header = f.readline().strip().split(',')
text_i = header.index('text')
has_url_i = header.index('has_url')
rows = []
for row_no, row in enumerate(f, start=2):
fields = row.rstrip('\n').split(',', has_url_i + 1)
txt = fields[text_i]
declared = bool(int(fields[has_url_i])) if fields[has_url_i] else False
detected = bool(p.search(txt))
rows.append((detected, declared))
same = sum(a == b for a, b in rows)
mismatches = [(detected, declared) for detected, declared in rows if detected != declared]
print("rows", len(rows))
print("same", same)
print("mismatches", len(mismatches))
print("mismatches_fraction", len(mismatches) / len(rows) if rows else 0)
for detected, declared in mismatches[:10]:
print("detected", detected, "declared", declared)
PYRepository: SafeFam/SafeFam_AI
Length of output: 272
🏁 Script executed:
#!/bin/bash
set -euo pipefail
csv="$(fd -t f 'phishing_total_dataset_2705.csv' . | head -n 1 || true)"
python3 - "$csv" <<'PY'
import re, sys, pathlib
path = sys.argv[1]
url_re = r"(?i)(?<!@)(?:https?://|www\.)[A-Za-z0-9\-._~:/?#\[\]@!$&'()*+;=%]+"
p = re.compile(url_re)
with open(path, encoding='utf-8-sig') as f:
header = f.readline().strip().split(',')
text_i = header.index('text')
has_url_i = header.index('has_url')
rows = []
for row_no, row in enumerate(f, start=2):
fields = row.rstrip('\n').split(',', has_url_i + 1)
txt = fields[text_i]
declared = bool(int(fields[has_url_i])) if fields[has_url_i] else False
detected = bool(p.search(txt))
rows.append((row_no, detected, declared))
same = sum((detected == declared) for _, detected, declared in rows)
mismatches = [row for _, detected, declared in rows if detected != declared]
print("rows", len(rows))
print("same", same)
print("mismatches", len(mismatches))
print("mismatches_fraction", len(mismatches) / len(rows) if rows else 0)
for row_no, detected, declared in mismatches[:10]:
print({"row": row_no, "detected": detected, "declared": declared})
PYRepository: SafeFam/SafeFam_AI
Length of output: 272
🏁 Script executed:
#!/bin/bash
set -euo pipefail
csv="$(fd -t f 'phishing_total_dataset_2705.csv' . | head -n 1 || true)"
python3 <<'PY' "$csv"
import re, sys, pathlib
path = sys.argv[1]
url_re = r"(?i)(?<!@)(?:https?://|www\.)[A-Za-z0-9\-._~:/?#\[\]@!$&'()*+;=%]+"
p = re.compile(url_re)
with open(path, encoding='utf-8-sig') as f:
header = f.readline().strip().split(',')
text_i = header.index('text')
has_url_i = header.index('has_url')
rows = []
for row_no, row in enumerate(f, start=2):
fields = row.rstrip('\n').split(',', has_url_i + 1)
txt = fields[text_i]
declared = bool(int(fields[has_url_i])) if fields[has_url_i] else False
detected = bool(p.search(txt))
rows.append((row_no, detected, declared))
same = sum((detected == declared) for _, detected, declared in rows)
mismatches = [row for _, detected, declared in rows if detected != declared]
print("rows", len(rows))
print("same", same)
print("mismatches", len(mismatches))
print("mismatches_fraction", len(mismatches) / len(rows) if rows else 0)
for row_no, detected, declared in mismatches[:10]:
print({"row": row_no, "detected": detected, "declared": declared})
PYRepository: SafeFam/SafeFam_AI
Length of output: 395
🏁 Script executed:
#!/bin/bash
set -euo pipefail
csv="$(fd -t f 'phishing total dataset 2705.csv' data_science/Data/SMSData | head -n 1 || true)"
python3 - "$csv" <<'PY'
import re, sys, pathlib
path = sys.argv[1]
url_re = r"(?i)(?<!@)(?:https?://|www\.)[A-Za-z0-9\-._~:/?#\[\]@!$&'()*+;=%]+"
p = re.compile(url_re)
with open(path, encoding='utf-8-sig') as f:
header = f.readline().strip().split(',')
print("csv", path)
print("header", header)
text_i = header.index('text')
has_url_i = header.index('has_url')
rows = []
for row_no, row in enumerate(f, start=2):
fields = row.rstrip('\n').split(',', has_url_i + 1)
txt = fields[text_i]
declared = bool(int(fields[has_url_i])) if fields[has_url_i] else False
detected = bool(p.search(txt))
rows.append((row_no, len(txt), detected, declared))
same = sum((detected == declared) for _, detected, declared in rows)
mismatches = [row for _, detected, declared in rows if detected != declared]
print("rows", len(rows))
print("same", same)
print("mismatches", len(mismatches))
print("mismatches_fraction", len(mismatches) / len(rows) if rows else 0)
for row_no, txt_len, detected, declared in mismatches[:10]:
print({"row": row_no, "text_length": txt_len, "detected": detected, "declared": declared})
PYRepository: SafeFam/SafeFam_AI
Length of output: 288
🏁 Script executed:
#!/bin/bash
set -euo pipefail
csv="$(fd -t f 'phishing_total_dataset_2705.csv' . | head -n 1 || true)"
python3 - "$csv" <<'PY'
import re, sys, pathlib
path = sys.argv[1]
url_re = r"(?i)(?<!@)(?:https?://|www\.)[A-Za-z0-9\-._~:/?#\[\]@!$&'()*+;=%]+"
p = re.compile(url_re)
with open(path, encoding='utf-8-sig') as f:
header = f.readline().strip().split(',')
print("csv", path)
print("header", header)
text_i = header.index('text')
has_url_i = header.index('has_url')
rows = []
for row_no, row in enumerate(f, start=2):
fields = row.rstrip('\n').split(',', has_url_i + 1)
txt = fields[text_i]
declared = bool(int(fields[has_url_i])) if fields[has_url_i] else False
detected = bool(p.search(txt))
rows.append((row_no, len(txt), detected, declared))
same = sum((detected == declared) for _, detected, declared in rows)
mismatches = [row for _, detected, declared in rows if detected != declared]
print("rows", len(rows))
print("same", same)
print("mismatches", len(mismatches))
print("mismatches_fraction", len(mismatches) / len(rows) if rows else 0)
for row_no, txt_len, detected, declared in mismatches[:10]:
print({"row": row_no, "text_length": txt_len, "detected": detected, "declared": declared})
PYRepository: SafeFam/SafeFam_AI
Length of output: 390
Reproduce the same has_url signal in single-message prediction.
Training and batch inference pass df["has_url"] to extract_struct_feature_matrix, but single-message paths call extract_struct_features(text) with no has_url argument. This sends 0 for URL flags when no regex match is found, even if the corresponding dataset value is True, so the model can learn a signal that serving cannot reproduce. Pass the same declared has_url value in naive_bayes_analyzer.py and in predict_risk_score, or derive it identically during training.
📍 Affects 2 files
app/analysis/text/naive_bayes_analyzer.py#L116-L119(this comment)data_science/SMSModel/train_sms.py#L546-L546
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/analysis/text/naive_bayes_analyzer.py` around lines 116 - 119, The
single-message feature construction in naive_bayes_analyzer.py and
predict_risk_score does not reproduce the declared has_url training signal.
Update each extract_struct_features call to pass the same has_url value used by
extract_struct_feature_matrix, including the site at
data_science/SMSModel/train_sms.py:546, or consistently derive that value
identically during training and serving.
| MANIFEST_COLUMNS = [ | ||
| "text_fingerprint", | ||
| "template_group_id", | ||
| "split", | ||
| "label", | ||
| "type", | ||
| ] |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Use DatasetSplitConfig when creating manifests.
DatasetSplitConfig supports custom key column names, but MANIFEST_COLUMNS always requires the default names. A non-default configuration can create valid splits, then fail in save_split_manifest with missing default columns.
Pass DatasetSplitConfig to build_split_manifest and save_split_manifest. Derive fingerprint, group, and label manifest columns from that config. Add a round-trip test with renamed columns.
Also applies to: 24-55
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@data_science/SMSModel/dataset_splitting/manifest.py` around lines 14 - 20,
Update the manifest-building and saving flow to accept and use
DatasetSplitConfig, including build_split_manifest and save_split_manifest,
instead of relying on fixed MANIFEST_COLUMNS names. Derive the fingerprint,
group, and label columns from the configuration so renamed key columns work end
to end, and add a round-trip test covering custom column names.
| for candidate_index in range(config.candidate_count): | ||
| splitter = GroupShuffleSplit( | ||
| n_splits=1, | ||
| test_size=selected_size, | ||
| random_state=( | ||
| config.random_state + random_state_offset + candidate_index | ||
| ), | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🌐 Web query:
For scikit-learn 1.8, does GroupShuffleSplit interpret test_size and train_size as proportions of groups rather than proportions of samples?
💡 Result:
Yes, in scikit-learn 1.8, GroupShuffleSplit interprets the test_size and train_size parameters as proportions of the number of unique groups, rather than as proportions of the total number of samples [1][2][3]. The official documentation explicitly states that these parameters refer to groups, not to samples, distinguishing its behavior from that of ShuffleSplit [2][3]. When a float value is provided for either parameter, it represents the proportion of groups to be included in the corresponding split (rounded up for test_size) [2][3]. If an integer value is provided, it represents the absolute number of groups to be included [2][3].
Citations:
- 1: https://sklearn.org/stable/modules/generated/sklearn.model_selection.GroupShuffleSplit.html
- 2: https://sklearn.org/1.8/modules/generated/sklearn.model_selection.GroupShuffleSplit.html
- 3: https://scikit-learn.org/1.8/modules/generated/sklearn.model_selection.GroupShuffleSplit.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Files matching splitter.py:\n'
fd -a 'splitter.py$' . || true
printf '\nGit tree around data_science/SMSModel/dataset_splitting:\n'
git ls-files data_science/SMSModel/dataset_splitting || true
if [ -f data_science/SMSModel/dataset_splitting/splitter.py ]; then
printf '\nFile size:\n'
wc -l data_science/SMSModel/dataset_splitting/splitter.py
printf '\nRelevant section:\n'
sed -n '1,180p' data_science/SMSModel/dataset_splitting/splitter.py | cat -n
fi
printf '\nSearch for score/select functions/GroupShuffleSplit:\n'
rg -n "_select_best_group_split|_candidate_score|GroupShuffleSplit|train_size|test_size|candidate_count|ratio" data_science/SMSModel/dataset_splitting/splitter.py || trueRepository: SafeFam/SafeFam_AI
Length of output: 7577
Select groups against the row-ratio target.
GroupShuffleSplit.test_size is a proportion of unique groups, not rows. Since _select_best_group_split fixes one group count before _candidate_score evaluates row ratios, unequal template group sizes will block the configured train, validation, and test row ratios. Generate candidates across feasible group counts, or use a whole-group optimizer that scores row count and label distribution together.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@data_science/SMSModel/dataset_splitting/splitter.py` around lines 85 - 92,
Update _select_best_group_split and its candidate-generation flow so candidates
vary across feasible group counts instead of fixing one count before scoring.
Ensure _candidate_score evaluates each whole-group split against the configured
train, validation, and test row ratios (while preserving label-distribution
scoring), allowing unequal group sizes to be optimized for row-level targets.
| def __post_init__(self) -> None: | ||
| values = np.asarray(self.values) | ||
| if values.ndim != 1: | ||
| raise ValueError("score values must be one-dimensional") | ||
| if not np.isfinite(values).all(): | ||
| raise ValueError("score values must contain only finite numbers") | ||
| if self.score_type == ScoreType.PROBABILITY and ( | ||
| (values < 0.0) | (values > 1.0) | ||
| ).any(): | ||
| raise ValueError("probability scores must be between 0 and 1") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Store the normalized NumPy array in values.
Line 29 validates np.asarray(self.values) but leaves self.values unchanged. A list input passes validation and later exposes a list instead of the required np.ndarray.
Proposed fix
def __post_init__(self) -> None:
- values = np.asarray(self.values)
+ values = np.asarray(self.values, dtype=float)
if values.ndim != 1:
raise ValueError("score values must be one-dimensional")
if not np.isfinite(values).all():
raise ValueError("score values must contain only finite numbers")
+ object.__setattr__(self, "values", values)📝 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.
| def __post_init__(self) -> None: | |
| values = np.asarray(self.values) | |
| if values.ndim != 1: | |
| raise ValueError("score values must be one-dimensional") | |
| if not np.isfinite(values).all(): | |
| raise ValueError("score values must contain only finite numbers") | |
| if self.score_type == ScoreType.PROBABILITY and ( | |
| (values < 0.0) | (values > 1.0) | |
| ).any(): | |
| raise ValueError("probability scores must be between 0 and 1") | |
| def __post_init__(self) -> None: | |
| values = np.asarray(self.values, dtype=float) | |
| if values.ndim != 1: | |
| raise ValueError("score values must be one-dimensional") | |
| if not np.isfinite(values).all(): | |
| raise ValueError("score values must contain only finite numbers") | |
| object.__setattr__(self, "values", values) | |
| if self.score_type == ScoreType.PROBABILITY and ( | |
| (values < 0.0) | (values > 1.0) | |
| ).any(): | |
| raise ValueError("probability scores must be between 0 and 1") |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@data_science/SMSModel/modeling/base.py` around lines 28 - 37, Update
__post_init__ to assign the normalized result of np.asarray(self.values) back to
self.values before validation, so list inputs are stored as NumPy arrays.
Preserve the existing dimensionality, finiteness, and probability-range checks.
| self._validate_dataframe( | ||
| train_df, | ||
| require_label=True, | ||
| ) | ||
|
|
||
| feature_matrix = self._build_feature_matrix( | ||
| train_df, | ||
| fit_vectorizer=True, | ||
| ) | ||
|
|
||
| base_model = ComplementNB( | ||
| alpha=self.alpha, | ||
| ) | ||
|
|
||
| calibrated_model = CalibratedClassifierCV( | ||
| estimator=base_model, | ||
| method=self.calibration_method, | ||
| cv=self.calibration_cv, | ||
| ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
According to the official scikit-learn documentation for CalibratedClassifierCV, what per-class sample-count constraint applies when cv is an integer?
💡 Result:
According to the scikit-learn implementation of CalibratedClassifierCV, when cv is an integer, the library imposes a constraint requiring at least as many samples per class as there are cross-validation folds [1][2]. Specifically, if the number of samples for any class is less than the specified number of folds (n_folds), the code raises a ValueError stating: "Requesting {n_folds}-fold cross-validation but provided less than {n_folds} examples for at least one class." [1][2] This check is performed to ensure that each cross-validation fold can potentially have at least one example per class, which is necessary for the stratified splitting strategies typically used [1][2].
Citations:
- 1: https://github.com/scikit-learn/scikit-learn/blob/master/sklearn/calibration.py
- 2: https://github.com/scikit-learn/scikit-learn/blob/fe2edb3cd/sklearn/calibration.py
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Locate file and relevant symbols"
git ls-files | rg '(^|/)naive_bayes\.py$' || true
echo
if [ -f data_science/SMSModel/modeling/naive_bayes.py ]; then
echo "File size:"
wc -l data_science/SMSModel/modeling/naive_bayes.py
echo
echo "Outline for NaiveBayesPhishingClassifier:"
ast-grep outline data_science/SMSModel/modeling/naive_bayes.py --view expanded 2>/dev/null || true
echo
echo "Relevant sections around validation/calibration and _validate_dataframe:"
sed -n '1,120p' data_science/SMSModel/modeling/naive_bayes.py
echo
sed -n '160,260p' data_science/SMSModel/modeling/naive_bayes.py
echo
echo "Calibration CV/default references:"
rg -n "calibration_cv|calibrated_model|_validate_dataframe|ComplementNB|CalibratedClassifierCV" data_science/SMSModel/modeling/naive_bayes.py
else
fd -a naive_bayes.py .
fi
echo
python3 - <<'PY'
try:
from sklearn.calibration import CalibratedClassifierCV
from sklearn.naive_bayes import ComplementNB
from sklearn.feature_extraction.text import TfidfVectorizer
from sklearn.linear_model import LogisticRegression
import numpy as np
X = np.array([["a"], ["b"]])
y = np.array([0, 1])
base = ComplementNB(alpha=1.0)
clf = CalibratedClassifierCV(estimator=base, method="prefit", cv=5)
clf.fit(X, y)
except Exception as e:
print(type(e).__name__ + ":", repr(str(e)))
PYRepository: SafeFam/SafeFam_AI
Length of output: 7471
🌐 Web query:
scikit-learn CalibratedClassifierCV constructor integer cv documentation each class examples constraint
💡 Result:
In scikit-learn, the cv parameter in the CalibratedClassifierCV constructor determines the cross-validation splitting strategy used for calibration [1][2]. When an integer is provided, it specifies the number of folds for the cross-validation process [1][3]. If cv is an integer or None (which defaults to 5-fold cross-validation), the following logic applies regarding the splitting strategy: - If the target variable y is binary or multiclass, the implementation uses StratifiedKFold to maintain class proportions across folds [1][4]. - If y is neither binary nor multiclass, it defaults to KFold [2][5]. There is no specific constructor constraint that requires a distinct cv value for "each class"; rather, the integer input dictates the total number of folds for the entire calibration procedure [1][4]. The splitting strategy is automatically determined based on the target variable's nature to ensure representative folds [5]. Example usage: from sklearn.calibration import CalibratedClassifierCV from sklearn.naive_bayes import GaussianNB # base_clf is your classifier, cv=3 sets 3-fold cross-validation calibrated_clf = CalibratedClassifierCV(base_clf, cv=3) calibrated_clf.fit(X, y) The cv parameter also accepts other inputs, including a cross-validation generator or an iterable that yields (train, test) splits, providing flexibility beyond a simple integer [1][2].
Citations:
- 1: https://scikit-learn.org/stable/modules/generated/sklearn.calibration.CalibratedClassifierCV
- 2: https://scikit-learn.org/1.9/modules/generated/sklearn.calibration.CalibratedClassifierCV.html
- 3: https://scikit-learn.org/stable/modules/generated/sklearn.calibration.CalibratedClassifierCV.html
- 4: https://github.com/scikit-learn/scikit-learn/blob/master/sklearn/calibration.py
- 5: https://github.com/scikit-learn/scikit-learn/blob/main/sklearn/calibration.py
🌐 Web query:
site:scikit-learn.org "CalibratedClassifierCV" "Requesting" "cross-validation" "at least one class"
💡 Result:
The error you are encountering—or the requirement to ensure "at least one class"—stems from how CalibratedClassifierCV performs cross-validation during the calibration process [1][2]. When using CalibratedClassifierCV, the model relies on cross-validation (by default, 5-fold StratifiedKFold for classification) to fit the base estimator and the calibrator [1][2][3]. For this process to function correctly, particularly when ensemble=True (the default), it is critical that all classes present in your dataset are represented in both the training and testing subsets of every cross-validation fold [2]. If a fold is generated where a specific class is entirely absent from the training or testing subset, the calibration for that class can fail or produce skewed results [2]. Specifically: 1. Training Set Absence: If a class is missing from the training subset of a fold, the classifier may not be able to produce meaningful probabilities for that class, often causing the predicted probability to default to 0 for that specific split [2]. 2. Test Set Absence: If a class is missing from the testing subset, the calibrator for that class cannot be properly fit, leading to ineffective calibration [2]. To resolve or prevent this issue: Ensure sufficient data: Verify that your dataset has enough samples per class so that each fold created by your cross-validation strategy (e.g., StratifiedKFold) contains at least one instance of every class [2][4]. Adjust the CV strategy: If you have very few samples for certain classes, you may need to reduce the number of folds (n_splits) in your cross-validation strategy to increase the likelihood that all classes appear in every fold [1][3]. Use cv="prefit": If you have a specific, pre-trained base estimator and a dedicated calibration dataset, use cv="prefit". This avoids cross-validation entirely, allowing you to use your prepared data for calibration without the risk of fold-related class omission [5][3]. For more details, consult the scikit-learn documentation on Probability Calibration [2].
Citations:
- 1: https://scikit-learn.org/stable/modules/generated/sklearn.calibration.CalibratedClassifierCV.html
- 2: https://scikit-learn.org/stable/modules/calibration.html
- 3: https://scikit-learn.org/0.23/modules/generated/sklearn.calibration.CalibratedClassifierCV.html
- 4: https://scikit-learn.org/stable/modules/generated/sklearn.model_selection.StratifiedKFold.html
- 5: https://scikit-learn.org/0.23/modules/calibration.html
Validate per-class sample count before calibration.
_validate_dataframe accepts one sample of each label, but integer CalibratedClassifierCV(cv=self.calibration_cv) requires at least that many samples per class. Add a pre-check on train_df["label"].value_counts().min() before fitting.
Proposed fix
self._validate_dataframe(
train_df,
require_label=True,
)
+ class_counts = train_df["label"].value_counts()
+ if class_counts.min() < self.calibration_cv:
+ raise ValueError(
+ "each training label must contain at least "
+ f"{self.calibration_cv} samples for calibration"
+ )
feature_matrix = self._build_feature_matrix(📝 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.
| self._validate_dataframe( | |
| train_df, | |
| require_label=True, | |
| ) | |
| feature_matrix = self._build_feature_matrix( | |
| train_df, | |
| fit_vectorizer=True, | |
| ) | |
| base_model = ComplementNB( | |
| alpha=self.alpha, | |
| ) | |
| calibrated_model = CalibratedClassifierCV( | |
| estimator=base_model, | |
| method=self.calibration_method, | |
| cv=self.calibration_cv, | |
| ) | |
| self._validate_dataframe( | |
| train_df, | |
| require_label=True, | |
| ) | |
| class_counts = train_df["label"].value_counts() | |
| if class_counts.min() < self.calibration_cv: | |
| raise ValueError( | |
| "each training label must contain at least " | |
| f"{self.calibration_cv} samples for calibration" | |
| ) | |
| feature_matrix = self._build_feature_matrix( | |
| train_df, | |
| fit_vectorizer=True, | |
| ) | |
| base_model = ComplementNB( | |
| alpha=self.alpha, | |
| ) | |
| calibrated_model = CalibratedClassifierCV( | |
| estimator=base_model, | |
| method=self.calibration_method, | |
| cv=self.calibration_cv, | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@data_science/SMSModel/modeling/naive_bayes.py` around lines 210 - 228, Before
constructing or fitting CalibratedClassifierCV in the training flow, use
train_df["label"].value_counts().min() to validate that every class has at least
self.calibration_cv samples. Raise the established validation error when the
minimum count is insufficient, while preserving _validate_dataframe and the
existing calibration setup for valid datasets.
| | Path | Purpose | | ||
| |---|---| | ||
| | `train_sms.py` | Naive Bayes training entry point | | ||
| | `artifacts/` | Versioned model and vectorizer files used by the API | | ||
| | `dataset_splitting/` | Leakage-safe train/validation/test splitting | | ||
| | `template_grouping/` | Duplicate and similar-message grouping | | ||
| | `splits/` | Reproducible split manifests | | ||
| | `reporting/` | Dataset report generation code | | ||
| | `reports/` | Generated summaries, metrics, and feature analysis data | | ||
| | `reports/figures/` | Generated plots and figures | | ||
| | `SMSDataModel.ipynb` | Exploratory analysis notebook | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add the new packages and the baseline runner to the workspace table.
This PR adds tokenization/, modeling/, evaluation/, and run_naive_bayes_baseline.py. The table omits all four, so the workspace map is already incomplete on merge.
📝 Proposed addition
| `train_sms.py` | Naive Bayes training entry point |
+| `run_naive_bayes_baseline.py` | Naive Bayes baseline evaluation entry point |
| `artifacts/` | Versioned model and vectorizer files used by the API |
| `dataset_splitting/` | Leakage-safe train/validation/test splitting |
| `template_grouping/` | Duplicate and similar-message grouping |
+| `tokenization/` | Kiwi morpheme tokenizer for vectorizers |
+| `modeling/` | Classifier contracts and Naive Bayes implementations |
+| `evaluation/` | Metrics, threshold selection, and latency measurement |
| `splits/` | Reproducible split manifests |📝 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.
| | Path | Purpose | | |
| |---|---| | |
| | `train_sms.py` | Naive Bayes training entry point | | |
| | `artifacts/` | Versioned model and vectorizer files used by the API | | |
| | `dataset_splitting/` | Leakage-safe train/validation/test splitting | | |
| | `template_grouping/` | Duplicate and similar-message grouping | | |
| | `splits/` | Reproducible split manifests | | |
| | `reporting/` | Dataset report generation code | | |
| | `reports/` | Generated summaries, metrics, and feature analysis data | | |
| | `reports/figures/` | Generated plots and figures | | |
| | `SMSDataModel.ipynb` | Exploratory analysis notebook | | |
| | Path | Purpose | | |
| |---|---| | |
| | `train_sms.py` | Naive Bayes training entry point | | |
| | `run_naive_bayes_baseline.py` | Naive Bayes baseline evaluation entry point | | |
| | `artifacts/` | Versioned model and vectorizer files used by the API | | |
| | `dataset_splitting/` | Leakage-safe train/validation/test splitting | | |
| | `template_grouping/` | Duplicate and similar-message grouping | | |
| | `tokenization/` | Kiwi morpheme tokenizer for vectorizers | | |
| | `modeling/` | Classifier contracts and Naive Bayes implementations | | |
| | `evaluation/` | Metrics, threshold selection, and latency measurement | | |
| | `splits/` | Reproducible split manifests | | |
| | `reporting/` | Dataset report generation code | | |
| | `reports/` | Generated summaries, metrics, and feature analysis data | | |
| | `reports/figures/` | Generated plots and figures | | |
| | `SMSDataModel.ipynb` | Exploratory analysis notebook | |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@data_science/SMSModel/README.md` around lines 3 - 13, Add tokenization/,
modeling/, evaluation/, and run_naive_bayes_baseline.py as rows in the workspace
table, with concise purposes matching their roles, so the README inventory
includes every newly added package and baseline runner.
| # Fail the image build early when the Kiwi wheel cannot be imported or its | ||
| # morphological analyzer cannot be initialized in the target architecture. | ||
| RUN python -c "from kiwipiepy import Kiwi; assert Kiwi().tokenize('installation check')" | ||
|
|
||
| COPY app app | ||
| COPY data_science/SMSModel/phishing_model_artifact.pkl models/phishing_model_artifact.pkl | ||
| COPY data_science/SMSModel/phishing_vectorizer.pkl models/phishing_vectorizer.pkl | ||
| COPY data_science/SMSModel/artifacts/phishing_model_artifact.pkl models/phishing_model_artifact.pkl | ||
| COPY data_science/SMSModel/artifacts/phishing_vectorizer.pkl models/phishing_vectorizer.pkl |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether the committed vectorizer artifact references the Kiwi tokenizer.
fd -t f 'phishing_vectorizer.pkl' --exec sh -c 'strings "$1" | grep -in "kiwi\|tokeniz" | head -n 20' _ {}Repository: SafeFam/SafeFam_AI
Length of output: 194
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Files of interest:\n'
git ls-files | grep -E '(^|/)(Dockerfile)$|phishing_vectorizer\.pkl$|kiwi_tokenizer\.py$|analysis/text/|smsmodel|SMSModel' | sed -n '1,200p'
printf '\nDockerfile relevant section:\n'
cat -n Dockerfile | sed -n '1,80p'
printf '\nKiwi tokenizer file:\n'
fd -t f 'kiwi_tokenizer\.py$' -x sh -c 'echo "--- $1"; cat -n "$1"' sh {}Repository: SafeFam/SafeFam_AI
Length of output: 14541
Copy the Kiwi tokenizer package into the runtime image.
data_science/SMSModel/tokenization/kiwi_tokenizer.py defines kiwi_tokenize, and the vectorizer model was built with that callable. The Dockerfile only runs KIWI during build, then copies app, the artifact files, and models, so data_science/SMSModel/tokenization/__init__.py and data_science/SMSModel/tokenization/kiwi_tokenizer.py are absent at runtime and model loading can fail with ModuleNotFoundError.
Copy the tokenization package into the image, or move kiwi_tokenize under a path already included at runtime.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Dockerfile` around lines 13 - 19, Ensure the runtime image includes the
module path required by the serialized vectorizer’s kiwi_tokenize callable.
Update the Dockerfile COPY steps to include
data_science/SMSModel/tokenization/__init__.py and kiwi_tokenizer.py, or
relocate kiwi_tokenize under an already-copied package while preserving its
import path at model load time.
| from data_science.SMSModel.train_sms import DATA_PATH, load_data | ||
|
|
||
| df_pool, df_holdout = load_data(DATA_PATH) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the unused holdout binding.
Line 307 assigns df_holdout but does not use it. Rename it to _df_holdout to satisfy Ruff RUF059.
🧰 Tools
🪛 Ruff (0.16.1)
[warning] 307-307: Unpacked variable df_holdout is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/data_science/SMSModel/test_template_grouping.py` around lines 305 -
307, Rename the unused holdout result binding in the load_data call to
_df_holdout, while preserving df_pool and the existing load_data invocation.
Source: Linters/SAST tools
1 participant
📝 개요
이 PR은 이슈 #36을 두 개의 PR로 나누어 진행하는 첫 번째 PR입니다.
기존 Naive Bayes 모델과 형태소 기반 Logistic Regression, 문자 n-gram
기반 Linear SVM을 동일한 조건에서 비교하기 위해서는 먼저 재현 가능한
데이터 분할과 공통 평가 기반이 필요합니다.
Logistic Regression, Linear SVM 및 최종 모델 비교는 후속 PR
(2/2)에서구현할 예정입니다.
🔗 관련 이슈
🎯 주요 변경 사항
1. 공통 SMS 전처리
2. 중복·유사 메시지 그룹화
template_group_id부여3. 데이터 누수 방지 분할
template_group_id단위 train/validation/test 분할생성 파일:
data_science/SMSModel/splits/sms_split_v1.csv4. 데이터 분할 검증 보고서
생성 파일:
data_science/SMSModel/reports/dataset_split_summary.jsondata_science/SMSModel/reports/dataset_split_summary.md5. 공통 모델 평가 파이프라인
다음 공통 인터페이스와 평가 기능을 추가했습니다.
fitpredictpredict_scores6. 기존 Naive Bayes baseline 연결
ComplementNB설정 유지평가 결과:
평가 보고서:
data_science/SMSModel/reports/model_evaluation/naive_bayes_baseline/7. Kiwi 형태소 tokenizer
kiwipiepy==0.23.2의존성 추가[URL],[ACCOUNT]등 마스킹 토큰 보존CountVectorizer연동 테스트데이터 분할 결과
1db45d5f2c3d3d17726888b05cd625e0d0a51deef3dc8ab94016a9ee97af18f5📸 사진
✅ PR 체크리스트
uvicorn구동 또는 테스트 코드)를 통과했습니다.Summary by CodeRabbit