Addressed copilot reviews

Author: VisLab

docs/search_implementation.md

@@ -53,18 +53,18 @@ It parses each raw HED string into a lightweight {class}`~hed.models.string_sear
 Key characteristics:
 
 - Input is a raw string (or a `pd.Series` via {func}`~hed.models.string_search.search_series`).
-- Schema is **optional**: pass a `schema_lookup` dict (see {mod}`hed.models.schema_lookup`) to enable ancestor matching for long-form strings; omit it for purely literal matching.
+- Schema is **optional**: pass a `schema_lookup` dict (see {mod}`hed.models.schema_lookup`) to enable ancestor matching for short-form strings (e.g. `Event` matching `Sensory-event`); omit it for purely literal matching.
 - Output is a list (truthy/falsy) — row-filtering only, no object references.
 - Supports the same full query syntax as `QueryHandler` (`&&`, `||`, `~`, `@`, `{}`, etc.).
 - `@A` carries the same semantics as `QueryHandler` — A must **not** be present.
-- Without a schema lookup, `Event` does **not** match `Sensory-event` (short-form strings). With a schema lookup, ancestor matching works for long-form strings (`Event/Sensory-event`).
+- Long-form strings (`Event/Sensory-event`) support ancestor matching via slash-splitting even without a lookup. Short-form strings (`Sensory-event`) require a `schema_lookup` for ancestor matching; without one, matching is purely literal.
 - Parse cost is a lightweight recursive split — much cheaper than a full HedString + schema parse.
 
 Use `StringQueryHandler` when you have raw strings (not `HedString` objects), need the full `QueryHandler` query syntax, and either don't have a schema available or want faster processing at the cost of losing full schema-aware ancestor matching. See {class}`hed.models.string_search.StringQueryHandler`.
 
 ### Generating a schema lookup
 
-If you want `StringQueryHandler` to resolve ancestors for long-form strings without a full schema parse per row, you can pre-generate a lookup dictionary from a `HedSchema`:
+If you want `StringQueryHandler` to resolve ancestors for short-form strings (e.g. query `Event` matching `Sensory-event`) without a full schema parse per row, you can pre-generate a lookup dictionary from a `HedSchema`:
 
 ```python
 from hed import load_schema_version

hed/models/schema_lookup.py

@@ -37,8 +37,6 @@
 import json
 from pathlib import Path
 
-from hed.schema.hed_schema_constants import HedSectionKey
-
 
 def generate_schema_lookup(schema):
     """Build a schema lookup table mapping short tag names to their ``tag_terms``.
@@ -69,7 +67,7 @@ def generate_schema_lookup(schema):
     # Handle HedSchemaGroup by iterating component schemas
     schemas = _iter_schemas(schema)
     for sch in schemas:
-        tags_section = sch._sections.get(HedSectionKey.Tags)
+        tags_section = sch.tags
         if tags_section is None:
             continue
         for name, entry in tags_section.items():
@@ -98,13 +96,9 @@ def _iter_schemas(schema):
     Yields:
         HedSchema: Individual schema objects.
     """
-    # HedSchemaGroup has a 'schemas' dict or list attribute
-    if hasattr(schema, "schemas"):
-        schemas_attr = schema.schemas
-        if isinstance(schemas_attr, dict):
-            yield from schemas_attr.values()
-        else:
-            yield from schemas_attr
+    # HedSchemaGroup stores member schemas in _schemas (a dict keyed by namespace)
+    if hasattr(schema, "_schemas"):
+        yield from schema._schemas.values()
     else:
         yield schema
 

tests/models/test_schema_lookup.py

@@ -189,6 +189,45 @@ def test_round_trip_preserves_tag_terms(self):
             os.unlink(tmp_path)
 
 
+class TestGenerateSchemaLookupGroup(unittest.TestCase):
+    """Verify that generate_schema_lookup works correctly with HedSchemaGroup."""
+
+    @classmethod
+    def setUpClass(cls):
+        base_data_dir = os.path.realpath(os.path.join(os.path.dirname(__file__), "../data/"))
+        hed_xml_file = os.path.join(base_data_dir, "schema_tests/HED8.0.0t.xml")
+        try:
+            from hed import schema as hed_schema
+            from hed.schema.hed_schema_group import HedSchemaGroup
+
+            single = hed_schema.load_schema(hed_xml_file)
+            cls.group = HedSchemaGroup([single])
+            cls.single_lookup = generate_schema_lookup(single)
+            cls.group_lookup = generate_schema_lookup(cls.group)
+        except Exception as exc:
+            cls.group = None
+            cls.single_lookup = None
+            cls.group_lookup = None
+            cls._setup_error = str(exc)
+
+    def _require_group(self):
+        if self.group_lookup is None:
+            self.skipTest(f"Schema not available: {getattr(self, '_setup_error', 'unknown error')}")
+
+    def test_group_returns_dict(self):
+        self._require_group()
+        self.assertIsInstance(self.group_lookup, dict)
+
+    def test_group_lookup_non_empty(self):
+        self._require_group()
+        self.assertGreater(len(self.group_lookup), 0)
+
+    def test_group_lookup_matches_single_lookup(self):
+        """A group wrapping a single schema should produce the same lookup as that schema alone."""
+        self._require_group()
+        self.assertEqual(self.group_lookup, self.single_lookup)
+
+
 class TestSchemaLookupUsedInSearch(TestSchemaLookupBase):
     """Integration tests: lookup dict plugged into StringQueryHandler."""
 
@@ -241,11 +280,9 @@ def items(self):
                 return []
 
         class _FakeSchema:
-            _sections = {}
-
             @property
-            def schemas(self):
-                return []
+            def tags(self):
+                return _FakeSection()
 
         result = generate_schema_lookup(_FakeSchema())
         self.assertIsInstance(result, dict)