Eliminate duplicate magic number weights and clarify security comment

[Copilot]

The default weights array was duplicated in three locations, and the security comment incorrectly stated that masking functions are used for coordinates when only a fixed redacted placeholder is logged.

Changes

Consolidated duplicate weights into documented constant

• Extracted [1.0, 1.0, -1.0, -1.0, 1.0, -1.0, 1.0, 1.0, 1.0, -1.0, 1.0] to module-level DEFAULT_WEIGHTS constant
• Added documentation explaining component-level weighting semantics (positive = increase influence, negative = down-weight)
• Replaced three inline occurrences with .copy() calls to prevent accidental mutation
• Changed argparse default from mutable list to None to avoid shared state

Corrected misleading security comment

• Updated comment from "coordinates are masked immediately" to "only a fixed redacted placeholder is written to the logs; no masking functions are used"
• Reflects actual implementation where _mask_latitude and _mask_longitude functions exist but are never called

# Before: duplicated in three locations
if weights is None:
    weights = [1.0, 1.0, -1.0, -1.0, 1.0, -1.0, 1.0, 1.0, 1.0, -1.0, 1.0]

# After: single source of truth with defensive copy
DEFAULT_WEIGHTS: List[float] = [1.0, 1.0, -1.0, -1.0, 1.0, -1.0, 1.0, 1.0, 1.0, -1.0, 1.0]

if weights is None:
    weights = DEFAULT_WEIGHTS.copy()

Original prompt

Please apply the following diffs and create a pull request.
Once the PR is ready, give it a title based on the messages of the fixes being applied.

[{"message":"The comment on line 322 states that coordinates are masked immediately, but the actual masking functions (_mask_latitude and _mask_longitude) are not called anywhere in the code. Consider either using the masking functions or updating the documentation to match the actual implementation.","fixFiles":[{"filePath":"src/spotforecast2_safe/tasks/task_safe_n_to_1_with_covariates_and_dataframe.py","diff":"diff --git a/src/spotforecast2_safe/tasks/task_safe_n_to_1_with_covariates_and_dataframe.py b/src/spotforecast2_safe/tasks/task_safe_n_to_1_with_covariates_and_dataframe.py\n--- a/src/spotforecast2_safe/tasks/task_safe_n_to_1_with_covariates_and_dataframe.py\n+++ b/src/spotforecast2_safe/tasks/task_safe_n_to_1_with_covariates_and_dataframe.py\n@@ -336,7 +336,8 @@\n         logger.info(f\"  Window Size: {window_size}\")\n         logger.info(f\"  Lags: {lags}\")\n         logger.info(f\"  Train Ratio: {train_ratio}\")\n-        # SECURITY: Do not log latitude/longitude (even masked) to avoid PII in logs (CWE-312, CWE-532)\n+        # SECURITY: Never log latitude/longitude at all to avoid PII in logs (CWE-312, CWE-532).\n+        #           Only a fixed redacted placeholder is written to the logs; no masking functions are used.\n         logger.info(\"  Location: [REDACTED]\")\n         # Log timezone region only, not full timezone (security: CWE-532)\n         logger.info(f\"  Region: {country_code}-{state}\")\n"}]},{"message":"The default weights array contains magic numbers without explanation. Consider defining this as a named constant with documentation explaining what each weight represents and why specific values are negative.","fixFiles":[{"filePath":"src/spotforecast2_safe/tasks/task_safe_n_to_1_with_covariates_and_dataframe.py","diff":"diff --git a/src/spotforecast2_safe/tasks/task_safe_n_to_1_with_covariates_and_dataframe.py b/src/spotforecast2_safe/tasks/task_safe_n_to_1_with_covariates_and_dataframe.py\n--- a/src/spotforecast2_safe/tasks/task_safe_n_to_1_with_covariates_and_dataframe.py\n+++ b/src/spotforecast2_safe/tasks/task_safe_n_to_1_with_covariates_and_dataframe.py\n@@ -51,6 +51,33 @@\n from spotforecast2_safe.manager.logger import setup_logging\n from spotforecast2_safe.manager.tools import _parse_bool\n \n+# Default aggregation weights for the N-to-1 forecasting task.\n+# \n+# The position of each value corresponds to a specific forecast component or\n+# aggregation term used by `agg_predict`. Positive values increase the influence\n+# of the corresponding component in the final aggregated forecast, whereas\n+# negative values down-weight or invert the contribution of that component.\n+# \n+# NOTE:\n+# - These defaults are domain-specific and should be updated together with any\n+#   changes to the aggregation logic or the ordering of components in\n+#   `agg_predict`.\n+# - They are defined as a named constant (rather than inline) to make it clear\n+#   what is being tuned and to avoid unexplained \"magic numbers\" in the code.\n+DEFAULT_AGGREGATION_WEIGHTS: List[float] = [\n+    1.0,\n+    1.0,\n+    -1.0,\n+    -1.0,\n+    1.0,\n+    -1.0,\n+    1.0,\n+    1.0,\n+    1.0,\n+    -1.0,\n+    1.0,\n+]\n+\n warnings.simplefilter(\"ignore\")\n \n \n@@ -324,7 +351,8 @@\n \n     # Default weights if not provided\n     if weights is None:\n-        weights = [1.0, 1.0, -1.0, -1.0, 1.0, -1.0, 1.0, 1.0, 1.0, -1.0, 1.0]\n+        # Use documented default aggregation weights instead of inline magic numbers.\n+        weights = DEFAULT_AGGREGATION_WEIGHTS\n \n     if verbose:\n         logger.info(\"=\" * 80)\n"}]},{"message":"Duplicate default weights definition. This violates DRY principle and creates maintenance burden when weights need to be updated. Extract to a module-level constant.","fixFiles":[{"filePath":"src/spotforecast2_safe/tasks/task_safe_n_to_1_with_covariates_and_dataframe.py","diff":"diff --git a/src/spotforecast2_safe/tasks/task_safe_n_to_1_with_covariates_and_dataframe.py b/src/spotforecast2_safe/tasks/task_safe_n_to_1_with_covariates_and_dataframe.py\n--- a/src/spotforecast2_safe/tasks/task_safe_n_to_1_with_covariates_and_dataframe.py\n+++ b/src/spotforecast2_safe/tasks/task_safe_n_to_1_with_covariates_and_dataframe.py\n@@ -53,7 +53,22 @@\n \n warnings.simplefilter(\"ignore\")\n \n+# Default weights used for prediction aggregation throughout this module.\n+DEFAULT_WEIGHTS: List[float] = [\n+    1.0,\n+    1.0,\n+    -1.0,\n+    -1.0,\n+    1.0,\n+    -1.0,\n+    1.0,\n+    1.0,\n+    1.0,\n+    -1.0,\n+    1.0,\n+]\n \n+\n def _mask_latitude(lat: float) -> str:\n     \"\"\"Mask latitude to 1 decimal place for safe logging (security: CWE-532, CWE-312).\n \n@@ -467,7 +481,7 @@\n         include_poly_features (bool): Toggle polynomial features. Default: False.\n         verbose (bool): Toggle detailed logging....

---

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

[bartzbeielstein]

copilot2

[codecov]

Codecov Report

❌ Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
.../task_safe_n_to_1_with_covariates_and_dataframe.py	66.66%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

[bartzbeielstein]

copilot 2