From 231d047ce954f7b3dfad0d52fd7b3dcc2d167096 Mon Sep 17 00:00:00 2001
From: Thomas Krijnen <t.krijnen@gmail.com>
Date: Wed, 10 Sep 2025 09:40:20 +0200
Subject: [PATCH] Capture SPF specific errors in validation test

---
 .../ifcopenshell/validate.py                  | 45 ++++++++++++++-----
 .../test/test_validate_rocksdb.py             | 12 ++++-
 2 files changed, 44 insertions(+), 13 deletions(-)

diff --git a/src/ifcopenshell-python/ifcopenshell/validate.py b/src/ifcopenshell-python/ifcopenshell/validate.py
index 2b8ad920f0..e1a88112ee 100644
--- a/src/ifcopenshell-python/ifcopenshell/validate.py
+++ b/src/ifcopenshell-python/ifcopenshell/validate.py
@@ -311,17 +311,22 @@ def assert_valid(
         return True
 
 
-def log_internal_cpp_errors(f: ifcopenshell.file, filename: str, logger: Union[Logger, json_logger]) -> None:
+def log_internal_cpp_errors(
+    f: Optional[ifcopenshell.file], filename: str, logger: Union[Logger, json_logger], log_content: Optional[str] = None
+) -> None:
     import re
     import bisect
 
     chr_offset_re = re.compile(r"at offset (\d+)\s*")
     for_instance_re = re.compile(r"\s*for instance #(\d+)\s*")
 
-    log = ifcopenshell.get_log()
-    msgs = list(map(json.loads, filter(None, log.split("\n"))))
+    if log_content is None:
+        log_content = ifcopenshell.get_log()
+    msgs = list(map(json.loads, filter(None, log_content.split("\n"))))
     chr_offsets = [chr_offset_re.findall(m["message"]) for m in msgs]
-    if chr_offsets:
+    instance_messages = [for_instance_re.findall(m["message"]) for m in msgs]
+
+    if chr_offsets or (instance_messages and f is None):
         # The file is opened in binary mode, in order
         # to correspond with the offsets reported by
         # IfcOpenShell C++
@@ -342,15 +347,25 @@ def log_internal_cpp_errors(f: ifcopenshell.file, filename: str, logger: Union[L
                 else:
                     logger.error("For instance:\n    %s\n%s", line, m)
 
-    instance_messages = [for_instance_re.findall(m["message"]) for m in msgs]
     if instance_messages:
         for instid, msg in zip(instance_messages, msgs):
             if instid:
                 m = for_instance_re.sub("", msg["message"])
-                try:
-                    inst = f[int(instid[0])]
-                except:
-                    inst = None
+                if f is not None:
+                    try:
+                        inst = f[int(instid[0])]
+                    except:
+                        inst = None
+                else:
+                    inst = next(
+                        (
+                            l
+                            for l in lines
+                            if l.strip().startswith(f"#{instid}".encode("ascii"))
+                            and re.sub(r"\s+", "", l).startswith(f"#{instid}=".encode("ascii"))
+                        ),
+                        None,
+                    )
                 if isinstance(logger, json_logger):
                     logger.set_state("instance", inst)
                     logger.set_state("attribute", None)
@@ -629,7 +644,7 @@ def to_string_header_entity(header_entity):
 def validate_ifc_header(
     f: Union[ifcopenshell.file, ifcopenshell.simple_spf.file], logger: Union[Logger, json_logger]
 ) -> None:
-    # @todo now that we have the header schema compiled into ifcopenshell, and the 
+    # @todo now that we have the header schema compiled into ifcopenshell, and the
     # header instances being conventional entity instances, this specific logic should
     # not be necessary anymore.
     header: Union[ifcopenshell.entity_instance, types.SimpleNamespace] = f.header
@@ -637,7 +652,11 @@ def validate_ifc_header(
     STRING_TYPE = "STRING (256)"
 
     def log_error(
-        header_entity: Union[ifcopenshell.entity_instance, tuple], name: str, index: int, expected_type: str, provided_type: str
+        header_entity: Union[ifcopenshell.entity_instance, tuple],
+        name: str,
+        index: int,
+        expected_type: str,
+        provided_type: str,
     ) -> None:
         logger.error(
             (
@@ -652,7 +671,9 @@ def validate_ifc_header(
             provided_type,
         )
 
-    def validate_attribute(header_entity: ifcopenshell.entity_instance, name: str, index: int, *, aggregate: bool = False) -> None:
+    def validate_attribute(
+        header_entity: ifcopenshell.entity_instance, name: str, index: int, *, aggregate: bool = False
+    ) -> None:
         try:
             value = getattr(header_entity, name)
         except RuntimeError as _:
diff --git a/src/ifcopenshell-python/test/test_validate_rocksdb.py b/src/ifcopenshell-python/test/test_validate_rocksdb.py
index dcd7029a1f..6c7c0eabc3 100644
--- a/src/ifcopenshell-python/test/test_validate_rocksdb.py
+++ b/src/ifcopenshell-python/test/test_validate_rocksdb.py
@@ -34,11 +34,21 @@ def test_file(file):
     logger = ifcopenshell.validate.json_logger()
     with tempfile.TemporaryDirectory() as d:
         rocks = os.path.join(d, os.path.basename(file) + ".rdb")
-        ifcopenshell.convert_path_to_rocksdb(file, rocks, errors=os.path.basename(file) + ".json")
+
+        # certain errors such as attribute counts / invalid enumeration literals
+        # are only captured during parsing of SPF as they are not represented in
+        # rocksdb, these errors need to be captured during conversion to rocksdb
+        # but can still be handled ifcopenshell.validate logger.
+        ifcopenshell.get_log()
+        ifcopenshell.ifcopenshell_wrapper.set_log_format_json()
+        ifcopenshell.convert_path_to_rocksdb(file, rocks)
+        log = ifcopenshell.get_log()
+
         try:
             ifcopenshell.validate.validate(rocks, logger)
         except ifcopenshell.SchemaError as e:
             pytest.skip()
+        # ifcopenshell.validate.log_internal_cpp_errors(None, file, logger, log_content=log)
         file = os.path.basename(file)
         if file.startswith("fail-"):
             assert len(logger.statements) > 0