fix: enhance error reporting by including HSN codes in error messages; remove unnecessary errors;

[karm1000]

Fixes: #3507

Summary by CodeRabbit

• Bug Fixes
  • GSTR‑1 error reporting now filters out empty entries and displays clearer messages, including HSN code details where applicable.
  • HSN summary errors are formatted consistently for improved readability.

[codecov]

Codecov Report

❌ Patch coverage is 16.66667% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.75%. Comparing base (0e6dd12) to head (62cdb8a).
⚠️ Report is 48 commits behind head on develop.

Files with missing lines	Patch %	Lines
...st_india/doctype/gst_return_log/generate_gstr_1.py	0.00%	8 Missing ⚠️
...mpliance/gst_india/utils/gstr_1/gstr_1_json_map.py	30.00%	7 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #3553      +/-   ##
===========================================
+ Coverage    60.98%   68.75%   +7.77%     
===========================================
  Files          134      182      +48     
  Lines        13788    17493    +3705     
===========================================
+ Hits          8409    12028    +3619     
- Misses        5379     5465      +86     

Files with missing lines	Coverage Δ
...mpliance/gst_india/utils/gstr_1/gstr_1_json_map.py	78.75% <30.00%> (-0.64%)	⬇️
...st_india/doctype/gst_return_log/generate_gstr_1.py	13.89% <0.00%> (-0.21%)	⬇️

... and 67 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

india_compliance/gst_india/utils/gstr_1/gstr_1_json_map.py (1)

1857-1866: Critical: error extraction breaks for categories that store lists (e.g., B2CS, NIL, AT/TXP).

data.values() may yield lists (e.g., B2CS groups invoices into lists by POS/Rate; NIL groups by type). Calling row.get(...) on a list raises AttributeError. This will crash convert_to_internal_data_format(..., for_errors=True) on such payloads.

Apply this diff to handle both dict and list rows:

-    errors = []
-    for category, data in output.items():
-        for row in data.values():
-            if not row.get("error_code") and not row.get("error_message"):
-                continue
-
-            row["category"] = category
-            errors.append(row)
+    errors = []
+    for category, data in output.items():
+        for rows in data.values():
+            candidates = rows if isinstance(rows, list) else [rows]
+            for row in candidates:
+                # skip non-dict entries defensively
+                if not isinstance(row, dict):
+                    continue
+                if not (row.get("error_code") or row.get("error_message")):
+                    continue
+                row["category"] = category
+                errors.append(row)

Consider adding a unit test for each list-based section (B2CS, NIL, AT/TXP) to ensure for_errors=True doesn’t throw and only returns rows with error_code or error_message.

🧹 Nitpick comments (3)

india_compliance/gst_india/utils/gstr_1/gstr_1_json_map.py (3)

1174-1175: Remove stdout debug print (“converting hsn summ”).

Printing in library code is noisy and hard to control in production. Use frappe.logger().debug(...) if needed, or drop it.

Apply this diff:

-        print("converting hsn summ")

---

1278-1284: Harden error formatter: remove prints, handle None, and make HSN injection idempotent.

• Avoid printing raw error_msg and data (may contain PII: invoice no., item details).
• Ensure the method doesn’t add the HSN twice if already present.
• Prefer the same suffix style used in GenerateGSTR1 (“... - HSN Code: ”) to stay consistent across code paths.

Apply this diff:

-    def get_error_message(self, error_msg, data=None):
-        print(error_msg, data, "\n\n\n")
-        if data and (hsn_code := data.get(inv_f.HSN_CODE)):
-            return f"HSN Code: {hsn_code} - {error_msg}".strip()
-
-        return error_msg
+    def get_error_message(self, error_msg, data=None):
+        # Normalize the base message
+        msg = (error_msg or "").strip()
+        if not data:
+            return msg
+        # Try both internal and gov keys to avoid ordering issues during formatting
+        hsn_code = data.get(inv_f.HSN_CODE) or data.get(gov_f.HSN_CODE)
+        if not hsn_code:
+            return msg
+        # Idempotency: if HSN already present, do nothing
+        if str(hsn_code) in msg:
+            return msg
+        # Match GenerateGSTR1 style (suffix)
+        return f"{msg} - HSN Code: {hsn_code}" if msg else f"HSN Code: {hsn_code}"

---

1843-1843: Drop module-level print in converter.

This will spam stdout whenever any conversion runs. Prefer frappe.logger().debug(...) gated by a debug flag, or remove.

Apply this diff:

-    print("Converting Gov data to internal format...\n\n")

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 9fa95201c6016ce0f012c7de466eecf79812ad05 and 62cdb8ab01c8bec4ced978ee589508cdcb61181c.

📒 Files selected for processing (2)

• india_compliance/gst_india/doctype/gst_return_log/generate_gstr_1.py (4 hunks)
• india_compliance/gst_india/utils/gstr_1/gstr_1_json_map.py (5 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• india_compliance/gst_india/doctype/gst_return_log/generate_gstr_1.py

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-04-22T13:15:18.792Z

Learnt from: Ninad1306
PR: resilient-tech/india-compliance#3222
File: india_compliance/gst_india/utils/gstr_1/gstr_1_json_map.py:1167-1171
Timestamp: 2025-04-22T13:15:18.792Z
Learning: In GSTR1 mappers like HSNSUM, serial number fields (num) don't need explicit entries in KEY_MAPPING when they're directly passed as GovDataField.INDEX.value to format_data(). Without a mapping entry, the format_data method preserves the original key (which is already correct) in the output.

Applied to files:

• india_compliance/gst_india/utils/gstr_1/gstr_1_json_map.py

🧬 Code graph analysis (1)

india_compliance/gst_india/utils/gstr_1/gstr_1_json_map.py (2)

india_compliance/gst_india/report/hsn_wise_summary_of_outward_supplies/hsn_wise_summary_of_outward_supplies.py (1)

• map_uom (258-271)

india_compliance/gst_india/utils/itc_04/itc_04_json_map.py (1)

• map_uom (44-53)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)

• GitHub Check: Codacy Static Code Analysis
• GitHub Check: Python Unit Tests
• GitHub Check: Mergify Merge Protections
• GitHub Check: Summary

[coderabbitai]

💡 Verification agent

🧩 Analysis chain

Make ERROR_MSG formatter effective and prevent double HSN injection.

• As-is, value_formatters_for_internal adds a formatter for gov_f.ERROR_MSG, but HSNSUM’s KEY_MAPPING does not map gov_f.ERROR_MSG → inv_f.ERROR_MSG (nor gov_f.ERROR_CD → inv_f.ERROR_CD). Without the mapping, downstream logic that reads row['error_message'] won’t see the formatted value, and your for_errors=True filter may miss errors originating at the row level.
• Also note: GenerateGSTR1 currently appends HSN to error messages for HSN rows. Combined with get_error_message here, that risks duplication (“HSN Code: 1102 - - HSN Code: 1102”).

Action:

• Add explicit mappings for ERROR_MSG and ERROR_CD in HSNSUM.KEY_MAPPING.
• Pick one place to inject HSN into error messages (here in HSNSUM or in GenerateGSTR1), and make the other path idempotent or a no-op to avoid duplicates.

Add these entries in HSNSUM.KEY_MAPPING (outside the edited hunk):

# In class HSNSUM.KEY_MAPPING
gov_f.ERROR_CD: inv_f.ERROR_CD,
gov_f.ERROR_MSG: inv_f.ERROR_MSG,

Optionally, keep only one of the following:

• Keep HSNSUM.get_error_message and make GenerateGSTR1 skip extra suffix if already present, or
• Remove HSNSUM formatter for ERROR_MSG and keep GenerateGSTR1’s augmentation.

Run this to confirm consistent mappings across mappers and to locate the GenerateGSTR1 augmentation:

---

🏁 Script executed:

#!/bin/bash
rg -nP --type=py 'KEY_MAPPING\s*=\s*\{(?:.|\n)*?ERROR_(MSG|CD)(?:.|\n)*?\}' -C2
rg -nP --type=py 'HSN Code:\s*\{?hsn_code\}?|HSN Code:' -C2

Length of output: 1236

---

Ensure ERROR_MSG and ERROR_CD are mapped and remove duplicate HSN injection

The current implementation adds a formatter for gov_f.ERROR_MSG but never maps it (or gov_f.ERROR_CD) in HSNSUM.KEY_MAPPING, and both HSNSUM.get_error_message and GenerateGSTR1 append the “HSN Code” prefix—resulting in duplicated HSN text.

• HSNSUM.KEY_MAPPING (class in gstr_1_json_map.py) has no entries for

gov_f.ERROR_CD → inv_f.ERROR_CD  
gov_f.ERROR_MSG → inv_f.ERROR_MSG

• HSNSUM.get_error_message (lines ~1279–1281 in india_compliance/gst_india/utils/gstr_1/gstr_1_json_map.py)
returns

f"HSN Code: {hsn_code} - {error_msg}"

• GenerateGSTR1 (lines ~583–587 in india_compliance/gst_india/doctype/gst_return_log/generate_gstr_1.py) also does

f"{error_message} - HSN Code: {hsn_code}"

Action items:

1. Add the missing mappings in HSNSUM.KEY_MAPPING:

   class HSNSUM:
       KEY_MAPPING = {
           …,
   +       gov_f.ERROR_CD: inv_f.ERROR_CD,
   +       gov_f.ERROR_MSG: inv_f.ERROR_MSG,
       }

2. Eliminate duplicate HSN injection by choosing one of:
   – Keep HSNSUM.get_error_message and guard in GenerateGSTR1 (e.g. skip suffix if already present), or
   – Remove the HSN prefix from HSNSUM.get_error_message and rely solely on GenerateGSTR1’s augmentation.