Address Timon review: derive metadata constants, require full parser shape

Derive SESSION_METADATA_FIELD_NAMES and SESSION_METADATA_REQUIRED_KEYS from
SessionMetadataDict introspection. Promote all finalize-produced fields to
required so mypy enforces completeness. Drive runtime validation from the
derived required-key set; update validation tests to use full metadata.

Author: clean6378-max-it

models/session.py

@@ -55,85 +55,50 @@ class MessageDict(TypedDict):
 class SessionMetadataDict(TypedDict):
     """Metadata accumulated while parsing a Claude Code JSONL session.
 
-    Required keys are always present after ``parse_session()``:
-
-    - ``session_id`` — derived from the ``.jsonl`` filename (stable identity).
-    - ``models_used`` — model names from assistant messages (empty list when none).
-    - ``first_timestamp`` — ISO timestamp of the earliest entry, or ``None`` when
-      the file has no timestamps.
-
-    Remaining fields are optional in partial or stub data (tests, export filters)
-    but are populated with defaults by the parser for full sessions.
+    ``parse_session()`` always produces every field below via
+    ``_finalize_session_metadata()``; defaults are zeros, empty collections,
+    or ``None`` where noted. Mypy treats the full shape as required so parser
+    and finalize code cannot drop a field silently.
+
+    The three identity/timing keys are also enforced at the runtime validation
+    boundary (``validate_session_dict``) with stricter type checks; remaining
+    keys must be present but are only type-checked lightly there.
     """
 
     session_id: str
     models_used: list[str]
     first_timestamp: str | None
-    last_timestamp: NotRequired[str | None]
-    total_input_tokens: NotRequired[int]
-    total_output_tokens: NotRequired[int]
-    total_cache_read_tokens: NotRequired[int]
-    total_cache_creation_tokens: NotRequired[int]
-    total_tool_calls: NotRequired[int]
-    tool_call_counts: NotRequired[dict[str, int]]
-    version: NotRequired[str | None]
-    cwd: NotRequired[str | None]
-    git_branch: NotRequired[str | None]
-    permission_mode: NotRequired[str | None]
-    compactions: NotRequired[int]
-    total_ephemeral_5m_tokens: NotRequired[int]
-    total_ephemeral_1h_tokens: NotRequired[int]
-    service_tiers: NotRequired[list[str]]
-    session_wall_time_seconds: NotRequired[float | None]
-    compact_boundaries: NotRequired[list[dict[str, Any]]]
-    api_errors: NotRequired[int]
-    files_read: NotRequired[list[str]]
-    files_written: NotRequired[list[str]]
-    files_created: NotRequired[list[str]]
-    bash_commands: NotRequired[list[Any]]
-    web_fetches: NotRequired[list[Any]]
-    sidechain_messages: NotRequired[int]
-    stop_reasons: NotRequired[dict[str, int]]
-    entry_counts: NotRequired[dict[str, int]]
-
-
-# Canonical metadata field set for parse_session builder / finalize parity.
-# Keep in sync with SessionMetadataDict above.
-SESSION_METADATA_REQUIRED_KEYS = frozenset({"session_id", "models_used", "first_timestamp"})
-
-SESSION_METADATA_FIELD_NAMES = frozenset(
-    {
-        "session_id",
-        "models_used",
-        "first_timestamp",
-        "last_timestamp",
-        "total_input_tokens",
-        "total_output_tokens",
-        "total_cache_read_tokens",
-        "total_cache_creation_tokens",
-        "total_tool_calls",
-        "tool_call_counts",
-        "version",
-        "cwd",
-        "git_branch",
-        "permission_mode",
-        "compactions",
-        "total_ephemeral_5m_tokens",
-        "total_ephemeral_1h_tokens",
-        "service_tiers",
-        "session_wall_time_seconds",
-        "compact_boundaries",
-        "api_errors",
-        "files_read",
-        "files_written",
-        "files_created",
-        "bash_commands",
-        "web_fetches",
-        "sidechain_messages",
-        "stop_reasons",
-        "entry_counts",
-    }
-)
+    last_timestamp: str | None
+    total_input_tokens: int
+    total_output_tokens: int
+    total_cache_read_tokens: int
+    total_cache_creation_tokens: int
+    total_tool_calls: int
+    tool_call_counts: dict[str, int]
+    version: str | None
+    cwd: str | None
+    git_branch: str | None
+    permission_mode: str | None
+    compactions: int
+    total_ephemeral_5m_tokens: int
+    total_ephemeral_1h_tokens: int
+    service_tiers: list[str]
+    session_wall_time_seconds: float | None
+    compact_boundaries: list[dict[str, Any]]
+    api_errors: int
+    files_read: list[str]
+    files_written: list[str]
+    files_created: list[str]
+    bash_commands: list[Any]
+    web_fetches: list[Any]
+    sidechain_messages: int
+    stop_reasons: dict[str, int]
+    entry_counts: dict[str, int]
+
+
+# Derived from SessionMetadataDict — single source of truth for parity tests.
+SESSION_METADATA_FIELD_NAMES = frozenset(SessionMetadataDict.__annotations__)
+SESSION_METADATA_REQUIRED_KEYS = SessionMetadataDict.__required_keys__
 
 
 class SessionMetadataBuilderDict(TypedDict):

tests/test_jsonl_validation.py

@@ -9,18 +9,28 @@
 sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
 
 from models.errors import SessionValidationError
-from utils.jsonl_parser import parse_session
+from utils.jsonl_parser import (
+    _finalize_session_metadata,
+    _new_session_metadata_builder,
+    parse_session,
+)
 from utils.validation import validate_session_dict
 
 FIXTURES = os.path.join(os.path.dirname(__file__), "fixtures")
 
 
+def _full_metadata(**overrides: Any) -> dict[str, Any]:
+    raw = _new_session_metadata_builder("abc123")
+    raw.update(overrides)
+    return dict(_finalize_session_metadata(raw))
+
+
 def _valid_payload(**overrides: Any) -> dict[str, Any]:
     base: dict[str, Any] = {
         "session_id": "abc123",
         "title": "Test Session",
         "messages": [{"role": "user", "text": "hello"}],
-        "metadata": {"session_id": "abc123", "models_used": [], "first_timestamp": None},
+        "metadata": _full_metadata(),
     }
     base.update(overrides)
     return base
@@ -61,45 +71,37 @@ def test_metadata_not_dict(self):
         assert exc_info.value.path == "metadata"
 
     def test_metadata_missing_session_id(self):
+        metadata = _full_metadata()
+        del metadata["session_id"]
         with pytest.raises(SessionValidationError) as exc_info:
-            validate_session_dict(
-                _valid_payload(metadata={"models_used": [], "first_timestamp": None})
-            )
+            validate_session_dict(_valid_payload(metadata=metadata))
         assert exc_info.value.path == "metadata.session_id"
 
     def test_metadata_missing_models_used(self):
+        metadata = _full_metadata()
+        del metadata["models_used"]
         with pytest.raises(SessionValidationError) as exc_info:
-            validate_session_dict(
-                _valid_payload(metadata={"session_id": "abc123", "first_timestamp": None})
-            )
+            validate_session_dict(_valid_payload(metadata=metadata))
         assert exc_info.value.path == "metadata.models_used"
 
     def test_metadata_missing_first_timestamp(self):
+        metadata = _full_metadata()
+        del metadata["first_timestamp"]
         with pytest.raises(SessionValidationError) as exc_info:
-            validate_session_dict(
-                _valid_payload(metadata={"session_id": "abc123", "models_used": []})
-            )
+            validate_session_dict(_valid_payload(metadata=metadata))
         assert exc_info.value.path == "metadata.first_timestamp"
 
     def test_metadata_first_timestamp_null_allowed(self):
         result = validate_session_dict(
-            _valid_payload(
-                metadata={"session_id": "abc123", "models_used": [], "first_timestamp": None}
-            )
+            _valid_payload(metadata=_full_metadata(first_timestamp=None))
         )
         assert result["metadata"]["first_timestamp"] is None
 
     def test_metadata_models_used_requires_string_elements(self):
+        metadata = _full_metadata()
+        metadata["models_used"] = ["claude-sonnet", 42]
         with pytest.raises(SessionValidationError) as exc_info:
-            validate_session_dict(
-                _valid_payload(
-                    metadata={
-                        "session_id": "abc123",
-                        "models_used": ["claude-sonnet", 42],
-                        "first_timestamp": None,
-                    }
-                )
-            )
+            validate_session_dict(_valid_payload(metadata=metadata))
         assert exc_info.value.path == "metadata.models_used[1]"
 
     def test_message_not_dict(self):

utils/validation.py

@@ -3,7 +3,7 @@
 from typing import Any, cast, get_args
 
 from models.errors import SessionValidationError
-from models.session import RoleLiteral, SessionDict
+from models.session import SESSION_METADATA_REQUIRED_KEYS, RoleLiteral, SessionDict
 
 _VALID_ROLES = frozenset(get_args(RoleLiteral))
 
@@ -58,13 +58,13 @@ def _require_str_list(path: str, val: Any) -> list[str]:
 
 
 def _validate_session_metadata(metadata: dict[str, Any]) -> None:
-    """Enforce SessionMetadataDict required keys at the runtime boundary."""
-    _require_field(metadata, "session_id", str, "str", path="metadata.session_id")
-    if "models_used" not in metadata:
-        raise SessionValidationError("metadata.models_used", "missing required field")
+    """Enforce SessionMetadataDict keys at the runtime boundary."""
+    for key in SESSION_METADATA_REQUIRED_KEYS:
+        if key not in metadata:
+            raise SessionValidationError(f"metadata.{key}", "missing required field")
+
+    _require_value("metadata.session_id", metadata["session_id"], str, "str")
     _require_str_list("metadata.models_used", metadata["models_used"])
-    if "first_timestamp" not in metadata:
-        raise SessionValidationError("metadata.first_timestamp", "missing required field")
     _require_optional_str("metadata.first_timestamp", metadata["first_timestamp"])