Fix yaml validator counter issuer

[dayantur]

This PR addresses issue #703 by changing terminal outputs and report messages to be in line with grid-id number rather than the internal site index counter.

Main changes

• Edited orchestrator.py to use gridiv for sites array instead of numeric index
• Edited phase_b_science_check.py to use gridiv for site identification instead of numeric index in report messages
• Edited phase_c_pydantic_report.py to use gridiv for site identification instead of numeric index in report messages
• Edited config.py to show gridiv number in terminal outputs from Pydantic model error messages

[github-actions]

🔍 Schema Preview Deployed

Preview URLs:

• 🆕 Preview schemas: https://umep-dev.github.io/SUEWS/preview/pr-709/schemas/suews-config/
• 📋 Preview landing: https://umep-dev.github.io/SUEWS/preview/pr-709/

Production URLs (unchanged):

• ✅ Production schemas: https://umep-dev.github.io/SUEWS/schemas/suews-config/
• 🏠 Production landing: https://umep-dev.github.io/SUEWS/

---

⚠️ Important: Preview schemas are in a subdirectory and do not affect production. The preview pages include warning banners to prevent accidental use in production configs.

[github-actions]

🤖 I've automatically formatted the code in this PR using:

• Python: ruff v0.8.6
• Fortran: fprettify v0.3.7

Please pull the latest changes before making further edits.

[sunt05]

Thanks, @dayantur – please take a look and see if these make sense.

[sunt05]

Thanks for working on this PR, @dayantur! The GRIDID improvements make error messages much more user-friendly. 🎉

After reviewing the code together with Claude, we identified an edge case in the string replacement approach in config.py:2221 that could cause issues when GRIDID values overlap with array indices.

The Issue:
Sequential string replacement can cause double-replacement. For example, if site 0 has GRIDID=1 and site 1 has GRIDID=200:

• Original: sites.0. and sites.1.
• After replacing 0→1: sites.1. and sites.1. ✓
• After replacing 1→200: sites.200. and sites.200. ❌ (both show 200!)

Suggested Solution:
Instead of string manipulation, work with Pydantic's structured error data (.errors()). This avoids collisions and handles arbitrary GRIDID values cleanly:

@classmethod
def _transform_validation_error(cls, error: ValidationError, config_data: dict) -> ValidationError:
    """Transform Pydantic validation errors to use GRIDID instead of array indices."""
    
    # Extract GRIDID mapping from sites
    sites = config_data.get("sites", [])
    site_gridid_map = {}
    for idx, site in enumerate(sites):
        if isinstance(site, dict):
            gridiv = site.get("gridiv")
            if isinstance(gridiv, dict) and "value" in gridiv:
                site_gridid_map[idx] = gridiv["value"]
            elif gridiv is not None:
                site_gridid_map[idx] = gridiv
            else:
                site_gridid_map[idx] = idx
        else:
            site_gridid_map[idx] = idx
    
    # Process structured errors (not string manipulation!)
    modified_errors = []
    for err in error.errors():
        err_copy = err.copy()
        loc_list = list(err_copy['loc'])
        
        # Replace numeric site index with GRIDID in location tuple
        if len(loc_list) >= 2 and loc_list[0] == 'sites' and isinstance(loc_list[1], int):
            site_idx = loc_list[1]
            if site_idx in site_gridid_map:
                loc_list[1] = site_gridid_map[site_idx]
        
        err_copy['loc'] = tuple(loc_list)
        modified_errors.append(err_copy)
    
    # Format into readable message
    error_lines = [f"{error.error_count()} validation error{'s' if error.error_count() > 1 else ''} for SUEWSConfig"]
    
    for err in modified_errors:
        loc_str = '.'.join(str(x) for x in err['loc'])
        error_lines.append(loc_str)
        error_lines.append(f"  {err['msg']} [type={err['type']}, input_value={err['input']}, input_type={type(err['input']).__name__}]")
        if 'url' in err:
            error_lines.append(f"    For further information visit {err['url']}")
    
    error_msg = '\n'.join(error_lines)
    raise ValueError(f"SUEWS Configuration Validation Error:\n{error_msg}")

Why this works:

• Processes error.errors() which returns structured error dictionaries
• Each error has a 'loc' tuple like ('sites', 0, 'properties', 'lat')
• Replace the numeric index in the tuple directly - no string collisions possible
• Handles arbitrary GRIDID values (strings, integers, anything)

Let me know if you'd like me to push this change or if you'd prefer to implement it! Either way, great work on improving the user experience with GRIDID-based error messages. 👏

[dayantur]

hi @sunt05 :) this looks good - please go ahead with your proposed solution and let me know if you need me to revise it before merging

[sunt05]

Then please implement that and I'll have a look later this afternoon - now need to go to some other meetings :)

[github-actions]

🤖 I've automatically formatted the code in this PR using:

• Python: ruff v0.8.6
• Fortran: fprettify v0.3.7

Please pull the latest changes before making further edits.

[github-actions]

🤖 I've automatically formatted the code in this PR using:

• Python: ruff v0.8.6
• Fortran: fprettify v0.3.7

Please pull the latest changes before making further edits.

[dayantur]

hi @sunt05 - I have addressed the changes you requested :) and implemented your solution for the buggy GRID ID problem with string replacement. This should now hopefully work as expected.

I also added some tests to cover the fixes.

[sunt05]

Well done! 🎉

The GRIDID transformation implementation looks excellent. The structured error processing approach correctly handles the edge case where GRIDID values could overlap with array indices, and you've provided comprehensive test coverage.

Key strengths:

• ✅ Proper use of Pydantic's structured error data (avoids string replacement collisions)
• ✅ Consistent GRIDID display across all validation phases (A, B, C)
• ✅ Test coverage for edge cases
• ✅ Graceful fallback to array indices when GRIDID unavailable

This will significantly improve the user experience for multi-site configurations. Approved!

[sunt05]

The CI is failing on the two new GRIDID collision tests. The issue is that both test sites have invalid data (lat: None), so both sites generate validation errors. When you have:

• Site 0 with GRIDID=1 and lat=None
• Site 1 with GRIDID=200 and lat=None

Both sites will generate 'lat' field required errors, so you'll see:

• sites.1.properties.lat (from site 0)
• sites.200.properties.lat (from site 1)

The test assertion self.assertEqual(error_msg.count('sites.1.'), 1) expects exactly 1 occurrence, but this is the correct behaviour - both sites should generate errors.

Fix: Make only ONE site invalid in each test:

def _create_invalid_config(self, gridid_values, invalid_site_index=0):
    """Create config with specified GRIDID values, only one site invalid."""
    sites = []
    for i, gid in enumerate(gridid_values):
        lat_value = None if i == invalid_site_index else 51.5  # Only first site invalid
        sites.append({'gridiv': gid, 'properties': {'lat': lat_value, 'lng': 0.0}})
    # ... rest of method

This ensures only site 0 (GRIDID=1 or 10) generates an error, so sites.1. and sites.10. appear exactly once.

[sunt05]

Updating my review status: I need to change my approval to request changes due to the failing tests.

@dayantur - Please update the test to make only one site invalid per test case. This will ensure the GRIDID appears exactly once in the error message, which is what the test expects.

Once the tests pass, I'll re-approve!

[sunt05]

I've fixed the failing tests! ✅

The issue was with the test logic - the tests were creating multiple invalid sites but expecting each GRIDID to appear only once in error messages.

Changes made:

1. Modified _create_invalid_config() to only make the first site invalid (lat=None), others valid (lat=51.5)
2. Updated test assertions to check:
   • ✅ Only invalid site's GRIDID appears in error (e.g., sites.1)
   • ✅ Valid site's GRIDID does NOT appear (e.g., sites.200)
   • ✅ Array index (sites.0) is properly replaced with GRIDID

All tests now pass locally. The CI should pass once the builds run.

[dayantur]

Thanks @sunt05 :) I already logged off when you spotted the failing test! Fingers crossed now for the fixed version