1️⃣ `_breakout_age_score()` में सीधा लॉजिक‑बग

- समस्या

```python

retest_info = assess_retest(weekly_df, level, confirmation_info)

if retest_info.get("status") == "retest_failed":

return 0

```

लेकिन `assess_retest()` ये keys लौटाता है:

```python

{"available": True, "result": "retest_successful" / "retest_failed" / ...}

```

यानी key `"status"` है ही नहीं, सही key `"result"` है। इस वजह से `retest_failed` वाला केस कभी ट्रिगर नहीं होगा, और फेल हुए retest के लिए भी age‑score 0 पर रीसेट नहीं होगा — ये आपके mean‑reversion / fade‑लॉजिक को चुपचाप गलत बना रहा है।

- सही समाधान (minimal fix)

```python

retest_info = assess_retest(weekly_df, level, confirmation_info)

if retest_info.get("result") == "retest_failed":

return 0

```

सुझाव: unit‑test लिखें जो `assess_retest(..., result="retest_failed")` वाले mock पर `_breakout_age_score()` से 0 आने को assert करे।

---

2️⃣ `describe_formation()` में `_intercept` uninitialized (potential crash)

- समस्या

```python

try:

slope, _intercept = np.polyfit(x, closes, 1)

except (np.linalg.LinAlgError, ValueError):

slope = 0.0

...

predicted = slope * x + _intercept

```

अगर `np.polyfit` exception फेंके (छोटा sample, collinearity आदि), तो `_intercept` कभी define नहीं होगा, और `predicted = ...` पर `UnboundLocalError` से function क्रैश करेगा।

- सही समाधान

`_intercept` को पहले से default पर सेट करें:

```python

avg_price = closes.mean()

x = np.arange(len(closes))

try:

slope, _intercept = np.polyfit(x, closes, 1)

except (np.linalg.LinAlgError, ValueError):

slope = 0.0

_intercept = avg_price # या 0.0, पर avg_price अधिक logical है

predicted = slope * x + _intercept

```

इससे regression fail होने पर भी graceful “range‑bound / no clear trend” टेक्स्ट safely निकलेगा, crash नहीं होगा।

---

3️⃣ `count_consolidation_weeks()` में range‑check की conceptual गलती

- समस्या (range_ok गलत तरीके से परिभाषित)

फंक्शन के docstring के मुताबिक, हर नए candidate‑window का पूरे window का High/Low anchor‑range के tolerance के भीतर होना चाहिए।

कोड:

```python

wk_high = float(window["High"].iloc[0])

wk_low = float(window["Low"].iloc[0])

range_ok = wk_high <= range_high (1 + tol) and wk_low >= range_low (1 - tol)

```

यहाँ `wk_high`/`wk_low` सिर्फ पहली candle के high/low हैं, पूरे window के नहीं।

इसका असर: अगर window के बीच में कोई spike हुआ हो, वो ignore हो जाएगा, जबकि logically consolidation टूट चुकी होती है।

- सही समाधान

पूरे window पर high/low लेना चाहिए:

```python

wk_high = float(window["High"].max())

wk_low = float(window["Low"].min())

range_ok = wk_high <= range_high (1 + tol) and wk_low >= range_low (1 - tol)

```

ये आपके V6b‑consolidation‑logic की documented definition से मेल खाएगा और false‑positives घटेंगे।

---

4️⃣ `detect_price_indicator_divergence()` के बाद dead / confusing कोड ब्लॉक

- समस्या

फंक्शन के अंदर आख़िर में ये pattern दिख रहा है:

```python

return result """ADX (Average Directional Index) — trend ki STRENGTH batata hai ..."""

adx, _, _ = calculate_adx_with_direction(df, period)

return adx

```

indentation के हिसाब से ये सब यही function के अंदर है, लेकिन पहले ही `return result` हो चुका है, तो ये पूरा block unreachable है (dead‑code)।

उपर से ये दिखता ऐसे है मानो `calculate_adx()` की docstring और body हों, पर असली `calculate_adx()` नीचे अलग से define है। इससे future maintenance में बहुत confusion होगा — कोई भी समझेगा कि divergence‑function के अंदर गलती से ADX का पुराना wrapper पड़ा है।

- सही समाधान

- इस unreachable block को पूरी तरह हटा दीजिए (docstring + दो लाइनें दोनों)।

- नीचे वाला `calculate_adx()` (RESTORE‑FIX वाला) ही single source of truth रहने दें।

Static‑analysis / linters (flake8, pylint) यहाँ “unreachable code” जैसा warning भी पकड़ेंगे; CI में इन्हें enable करना अच्छा रहेगा।

---

5️⃣ `NSELIB_AVAILABLE` दो बार define — semantic तो सही, design खराब

- समस्या

आपने दो जगह पर try/except किया है:

```python

try:

from nselib import derivatives as nse_derivatives

NSELIB_AVAILABLE = True

except ImportError:

NSELIB_AVAILABLE = False

...

try:

from nselib import capital_market

NSELIB_AVAILABLE = True

except ImportError:

NSELIB_AVAILABLE = False

```

practical रूप से ये ठीक है (दोनों import fail होंगे तो False, किसी एक के success पर True), लेकिन:

- global flag का मतलब ambiguous हो जाता है: “derivatives available?” या “capital_market available?”

- future में अगर सिर्फ derivatives fail और capital_market pass हुआ तो भी NSELIB_AVAILABLE=True रहेगा, जबकि option‑chain वगैरह fail करेंगे।

- बेहतर स्ट्रक्चर

```python

try:

from nselib import derivatives as nse_derivatives

NSELIB_DERIVATIVES_AVAILABLE = True

except ImportError:

NSELIB_DERIVATIVES_AVAILABLE = False

try:

from nselib import capital_market

NSELIB_CM_AVAILABLE = True

except ImportError:

NSELIB_CM_AVAILABLE = False

NSELIB_AVAILABLE = NSELIB_DERIVATIVES_AVAILABLE or NSELIB_CM_AVAILABLE

```

और हर जगह dependency‑specific flag इस्तेमाल करें (option‑chain → `NSELIB_DERIVATIVES_AVAILABLE` आदि)।

---

6️⃣ DataFrame mutation pattern – side‑effects का structural risk

- समस्या

कई helper functions input `df` को सीधे mutate कर रहे हैं:

- `calculate_dma`, `calculate_ema`, `calculate_bollinger_bands`, `calculate_macd`, `calculate_rsi`, `calculate_supertrend` आदि सब original `df` में ही नए columns जोड़ते हैं।

- कई जगह caller `df.copy()` देता है (सही), लेकिन कुछ जगह raw `df` पास हो सकता है तो unintended columns add हो जाएंगे → downstream logic में गड़बड़ी / memory blow‑up / debugging मुश्किल।

- सुझाव (structural सुधार)

कम से कम core‑library level पर दो principles follow करें:

1. Pure functions:

- या तो function हमेशा `df.copy()` पर काम करके नया DataFrame return करे,

- या contract documentation में साफ़ लिखें: “यह function in‑place mutate करता है।”

2. High‑level orchestration जैसे `run_technical_analysis`, `run_price_action_analysis` हमेशा `history.copy()` भेजें (आपने कई जगह किया है, पर enforce करना ज़रूरी है)।

Unit‑tests में ये check करें कि “फ्लो में किसी intermediate call के बाद extra unintended columns नहीं बढ़ रहे” (या consciously allowed list हो)।

---

7️⃣ Single‑file monolith – maintainability / testability पर बड़ी structural कमी

- समस्या

अभी पूरा सिस्टम (symbol utils, NSE scraping, technicals, risk, decision‑engine, backtest helpers, CLI) एक ही फाइल में है (~हज़ारों लाइनों का)। इसके नुकसान:

- Local reasoning मुश्किल (एक जगह change का effect बहुत दूर तक हो सकता है)।

- Unit‑tests granular level पर लिखना कठिन।

- Import‑time side‑effects (global caches, NSE calls, KOTAK_AVAILABLE detection इत्यादि) debug करना मुश्किल।

- Python module reload / hot‑reload के समय random bugs आने की संभावना।

- बेहतर architecture (recommended refactor)

Development repo में structure ऐसा रखें (और ज़रूरत हो तो build‑step से single‑file bundle बनाएँ):

- `symbol_utils.py` → resolve_symbol, INDEX_MAP, get_live_price_only इत्यादि

- `ta_core.py` → सारी pure technical functions (DMA/EMA/ATR/ADX/RSI/Bollinger/ATR‑compression आदि)

- `patterns_breakout.py` → breakout_levels, clusters, consolidation, mean‑reversion engine, 20‑DMA predictor

- `risk_derivatives.py` → option‑chain, HV, premium‑valuation, VaR

- `ownership_nse.py` → pledging, bulk/block, delivery, promoter holding

- `fundamentals.py` → valuation / quality / growth / dividends

- `decision_engine.py` → scoring, safety filters, recommendation, explanations

- `nse_clients.py` / `kotak_client.py` → सारा HTTP/session logic और tokens

- `main_cli.py` → सिर्फ `analyze()` और `print_report()`

फिर `stock_agent.py` auto‑generated “single‑file” हो सकता है (build script से modules को concatenate करके), पर development और testing हमेशा modular code पर होनी चाहिए।

---

8️⃣ Error‑handling overly broad — debugging में दिक्कत

- समस्या

बहुत जगह `except Exception: pass` या generic `"read_error"` / `"NSE_direct_call_fail"` कर के swallow कर दिया जाता है (logging सिर्फ कभी‑कभी है)।

इससे production‑में subtle bugs silent रहते हैं, जैसे:

- NSE schema बदल जाए, कुछ fields miss हो जाएँ → आप सिर्फ “available: False” देखेंगे, root‑cause नहीं।

- बाहरी लाइब्रेरी की breaking‑change तुरन्त दिखेगी नहीं।

- सुझाव

- Low‑level helper में कम से कम `print()` या structured log करें (आपने कुछ जगह किया भी है, वही pattern uniformly follow करें):

- endpoint, HTTP‑status, response‑shape, short exception message।

- High‑level पर user ko friendly message दे सकते हैं, लेकिन internals में exact error capture ज़रूर कीजिए (file‑based या console‑based log)।

- NSE‑scrapers के लिए एक centralized `_nse_request()` helper बनाकर उसी में retry + error‑logging रखेंगे तो duplication भी घटेगा।

---

9️⃣ Testing‑related gaps (high‑risk logic पर कोई guard नहीं)

आपने बहुत ईमानदारी से हर function पर “HONEST: UNVALIDATED / backtest‑pending” लिख रखा है — ये अच्छा है, लेकिन practically risk ये है कि future refactor में कोई भी इन functions को signal‑engine में ज़्यादा weight दे दे, और bug unnoticed रहे।

Structural सुधार के सुझाव:

- कम से कम इन critical blocks पर unit‑tests अनिवार्य रखें:

- breakout/cluster logic (`get_breakout_levels`, `detect_resistance_support_clusters`)

- mean‑reversion और 20‑DMA‑touch predictor (features, threshold, direction‑handling)

- pledge / bulk / delivery NSE‑scrapers (fixed JSON samples पर parsing tests)

- हर validated‑backtest वाले part के लिए tests में sample‑cases hard‑code करें (जैसे ATR/ADX RMA vs SMA वाली fixes के बाद)।

---