Skip to content

feat(validation): add topic suffix validation utilities and contract schema [OMN-1537] - #435

Merged
jonahgabriel merged 8 commits into
mainfrom
jonah/omn-1537-update-kafka-topic-constants-to-onex-naming-convention
Jan 26, 2026
Merged

jonahgabriel merged 8 commits into
mainfrom
jonah/omn-1537-update-kafka-topic-constants-to-onex-naming-convention

Conversation

@jonahgabriel

@jonahgabriel jonahgabriel commented Jan 25, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Add ONEX topic suffix validation for contract-driven topic declaration.

Key Principle: Core defines convention, not a global topic registry. Domain repos declare topics in contracts.

Changes

  • Add validate_topic_suffix(), parse_topic_suffix(), compose_full_topic() utilities
  • Add ModelTopicSuffixParts and ModelTopicValidationResult models
  • Add publish_topics/subscribe_topics fields to ModelEventBusSubcontract
  • Add ModelTopicMeta for future schema_ref support
  • Add TOKEN_TO_TOPIC_TYPE mapping and EnumTopicType.DLQ
  • Add 174 comprehensive tests

Topic Suffix Format

onex.{kind}.{producer}.{event-name}.v{n}

Examples:

  • onex.evt.omnimemory.intent-stored.v1
  • onex.cmd.omnimemory.intent-query-requested.v1

Full topic composed at runtime: {env}.{suffix} → dev.onex.evt.omnimemory.intent-stored.v1

Test Plan

  • 174 unit tests covering valid/invalid suffix patterns
  • All pre-commit hooks pass
  • mypy strict mode passes
  • CI passes

Follow-up Tickets

  • OMN-1550: Runtime topic resolution from contracts
  • OMN-1538: omnimemory contract migration
  • OMN-1551: omniintelligence contract migration
  • OMN-1552: omniclaude audit
  • OMN-1553: omnidash audit
  • OMN-1554: Core legacy constant cleanup

Summary by CodeRabbit

  • New Features

    • Dead Letter Queue (DLQ) added as a new topic type with a ready-made DLQ topic configuration.
    • Canonical topic-suffix utilities: validate, parse, compose, and is-valid helpers.
    • Topic metadata model and event-bus contract fields to declare publish/subscribe topics and per-topic metadata.
    • Utilities to map topic tokens ↔ topic types and list valid suffix kinds.
  • Tests

    • Extensive unit tests for validator, parsing, mappings, models, edge cases, and thread-safety.

✏️ Tip: You can customize this high-level summary in your review settings.

…schema [OMN-1537]

Add ONEX topic suffix validation for contract-driven topic declaration:

- Add validate_topic_suffix(), parse_topic_suffix(), compose_full_topic()
- Add ModelTopicSuffixParts and ModelTopicValidationResult models
- Add publish_topics/subscribe_topics fields to ModelEventBusSubcontract
- Add ModelTopicMeta for future schema_ref support
- Add TOKEN_TO_TOPIC_TYPE mapping and EnumTopicType.DLQ
- Add 174 comprehensive tests for validation utilities

Topic suffix format: onex.{cmd|evt|dlq|snapshot}.{producer}.{event-name}.v{n}
Full topic composed at runtime: {env}.{suffix}

Core owns grammar and validation; domain repos declare topics in contracts.
@coderabbitai

coderabbitai Bot commented Jan 25, 2026 •

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

Adds DLQ topic type and token↔type accessors; introduces immutable topic-meta model; implements ONEX topic-suffix validator (parse/validate/compose/is_valid) with models and exports; extends event-bus subcontract with publish/subscribe topic lists, per-topic metadata and validation; wires DLQ into manifests, defaults, and tests.

Changes

Cohort / File(s) Summary
Topic taxonomy & exports
src/omnibase_core/enums/enum_topic_taxonomy.py, src/omnibase_core/constants/constants_topic_taxonomy.py, src/omnibase_core/constants/__init__.py
Add DLQ / TOPIC_TYPE_DLQ; include DLQ in valid types and exports; add lazy token↔type mappings and accessors get_token_to_topic_type, get_topic_type_to_token, get_valid_topic_suffix_kinds.
Validator implementation & package surface
src/omnibase_core/validation/validator_topic_suffix.py, src/omnibase_core/validation/__init__.py, src/omnibase_core/validation/*
New ONEX topic-suffix validator: constants/patterns, ENV_PREFIXES, EXPECTED_SEGMENT_COUNT, KEBAB/VERSION/TOPIC regexes, and functions validate_topic_suffix, parse_topic_suffix, compose_full_topic, is_valid_topic_suffix; re-exported at package level.
Validation models & tokens
src/omnibase_core/models/validation/model_topic_suffix_parts.py, src/omnibase_core/models/validation/model_topic_validation_result.py, src/omnibase_core/models/validation/__init__.py
Add immutable Pydantic models ModelTopicSuffixParts and ModelTopicValidationResult (with factories, invariants, and forward-ref handling); introduce TOPIC_KIND_* tokens and VALID_TOPIC_KINDS; export symbols.
Event bus subcontract & topic metadata
src/omnibase_core/models/contracts/subcontracts/model_event_bus_subcontract.py, src/omnibase_core/models/contracts/subcontracts/model_topic_meta.py, src/omnibase_core/models/contracts/subcontracts/__init__.py
Add publish_topics/subscribe_topics and optional per-topic metadata (ModelTopicMeta); add field validator validate_topic_suffixes to validate suffix list entries; re-export ModelTopicMeta.
Topic manifest & defaults
src/omnibase_core/models/events/model_topic_config.py, src/omnibase_core/models/events/model_topic_manifest.py
Add ModelTopicConfig.dlq_default() and include DLQ in standard and registration manifest topic maps and name generation.
Top-level validation & model exports
src/omnibase_core/models/validation/__init__.py, src/omnibase_core/validation/__init__.py
Re-export validator functions, constants, validation models, and tokens at package level (extend __all__).
Constants & tests
src/omnibase_core/constants/__init__.py, tests/unit/validation/test_validator_topic_suffix.py, tests/unit/constants/test_constants_topic_taxonomy.py, tests/unit/models/events/test_model_topic_manifest.py
Add tests covering validator behavior, parsing, composition, patterns, edge cases, thread-safety, immutability, and inclusion of DLQ across mappings/manifest; update constant exports and tests to include TOPIC_TYPE_DLQ.

Sequence Diagram(s)

sequenceDiagram
    participant App as Application Code
    participant Validator as validator_topic_suffix
    participant Enum as EnumTopicType
    participant Model as ModelTopicValidationResult
    App->>Validator: validate_topic_suffix(suffix)
    activate Validator
    Validator->>Validator: normalize, split, check segments/patterns
    alt invalid structure or env/pattern
        Validator->>Model: failure(suffix, error)
        Model-->>App: {is_valid: false, error}
    else structure OK
        Validator->>Enum: verify kind ∈ VALID_TOPIC_KINDS
        alt invalid kind
            Validator->>Model: failure(suffix, "invalid kind")
            Model-->>App: {is_valid: false, error}
        else kind OK
            Validator->>Validator: validate producer/event/version formats
            Validator->>Model: success(suffix, parsed_parts)
            Model-->>App: {is_valid: true, parsed}
        end
    end
    deactivate Validator
Loading
sequenceDiagram
    participant App as Application Code
    participant Composer as compose_full_topic
    participant Validator as validator_topic_suffix
    App->>Composer: compose_full_topic(env_prefix, suffix)
    activate Composer
    Composer->>Validator: is_valid_topic_suffix(suffix)
    Validator-->>Composer: boolean
    alt invalid suffix
        Composer-->>App: raise ValueError
    else valid suffix
        Composer->>Composer: validate env_prefix ∈ ENV_PREFIXES
        alt invalid env
            Composer-->>App: raise ValueError
        else
            Composer-->>App: return "{env_prefix}.{suffix}"
        end
    end
    deactivate Composer
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

Poem

🥕 I hopped through tokens, tiny and spry,

DLQ popped up with a curious eye.
Parsers nibble parts, tidy and bright,
Validators guard each kebab-case bite.
Rabbit cheers — topics validated tonight.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title clearly and specifically describes the main changes: adding topic suffix validation utilities and contract schema with the associated ticket reference.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Comment @coderabbitai help to get the list of available commands and usage tips.

…ation [OMN-1537]

- Add TOPIC_KIND_INTENT constant to model_topic_suffix_parts.py
- Update VALID_TOPIC_KINDS to include all 5 kinds (cmd, evt, dlq, intent, snapshot)
- Update regex pattern to match intent topics
- Add cross-reference documentation between topic constant modules
- Add 13 unit tests for token mapping functions
- Update test expectations to include intent kind

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🤖 Fix all issues with AI agents
In `@src/omnibase_core/constants/constants_topic_taxonomy.py`:
- Around line 62-70: Remove the inconsistent "intent" entry from the
token_to_type mapping so the canonical topic-kind set matches the validator and
ModelTopicSuffixParts; update token_to_type (the dict mapping token_to_type) to
only include "cmd", "dlq", "evt", and "snapshot", regenerate type_to_token
(which is built from token_to_type) and valid_kinds so they reflect the same
four tokens, ensuring EnumTopicType values referenced remain consistent and
get_valid_topic_suffix_kinds() no longer exposes "intent".

In
`@src/omnibase_core/models/contracts/subcontracts/model_event_bus_subcontract.py`:
- Around line 225-236: Replace the ValueError raise in the
validate_topic_suffixes class validator with a ModelOnexError using
EnumCoreErrorCode.VALIDATION_ERROR: import ModelOnexError and EnumCoreErrorCode
at top of the function (or module) and when validate_topic_suffix(topic) returns
not is_valid raise ModelOnexError(EnumCoreErrorCode.VALIDATION_ERROR,
context={"field":"publish_topics/subscribe_topics","invalid_value":topic,"detail":result.error})
so ONEX-specific consumers can handle the validation failure consistently; keep
the loop and return topics as-is.

In `@src/omnibase_core/models/validation/model_topic_suffix_parts.py`:
- Around line 1-37: The module docstring is too long; shorten it to a concise
3–6 line summary describing the module purpose (parsed ONEX topic suffix parts)
and the topic format, then move the extended example, thread-safety note, and
"See Also" details into the ModelTopicSuffixParts class docstring (or external
docs) — update references to ModelTopicSuffixParts, validator_topic_suffix, and
ModelTopicValidationResult to ensure they remain documented in the class
docstring or docs.

In `@src/omnibase_core/models/validation/model_topic_validation_result.py`:
- Around line 1-34: The module docstring is overly long for this simple model;
shorten it to a concise 3–6 line summary and move the detailed examples and
extended notes into the ModelTopicValidationResult class docstring or external
docs. Edit the top-level docstring in the file to a brief one- or two-sentence
description of the module's purpose (e.g., "Pydantic model for ONEX topic suffix
validation results"), and relocate the example usage, thread-safety note, and
"See Also" references into the ModelTopicValidationResult class docstring (or
project docs) while keeping references to validator_topic_suffix and
ModelTopicSuffixParts there for discoverability.

In `@src/omnibase_core/validation/validator_topic_suffix.py`:
- Around line 100-268: The function validate_topic_suffix currently returns the
lowercased "normalized" value in ModelTopicValidationResult.success which
conflicts with the docstring claiming the result holds the original suffix; fix
by returning the trimmed original (use the local variable stripped, not
normalized) when calling ModelTopicValidationResult.success (and adjust any
success/failure returns that should reflect the trimmed original), and update
the docstring to state that the returned suffix is the input trimmed of
whitespace (or, alternatively, explicitly document that results are normalized
if you prefer that behavior) — target symbols: validate_topic_suffix,
normalized, stripped, ModelTopicValidationResult.success (and any
ModelTopicValidationResult.failure calls you decide should use the trimmed
original).

In `@tests/unit/validation/test_validator_topic_suffix.py`:
- Around line 671-760: Tests include three occurrences of inline type ignores
(e.g., in assignments and constructor calls in
TestModelTopicSuffixParts.test_model_is_frozen, test_model_forbids_extra_fields,
test_all_fields_required and in
TestModelTopicValidationResult.test_model_is_frozen) that require a ticketed
justification; update each "# type: ignore[...]" to include a short explanatory
comment with a Linear ticket reference (e.g., "# type: ignore[call-arg]  #
LINEAR-<number>: reason") immediately adjacent to the ignore, explaining why the
ignore is necessary and linking to the ticket, and ensure the symbols referenced
are the ModelTopicSuffixParts constructor usage and the
ModelTopicValidationResult instance mutation so reviewers can trace the
justification.
🧹 Nitpick comments (3)
src/omnibase_core/constants/constants_topic_taxonomy.py (1)

90-139: Avoid exposing mutable cached mappings.
The accessors return cached dicts directly, so callers can mutate shared state. Return defensive copies (or mapping proxies) to keep the cache immutable.

♻️ Suggested change (defensive copies)
 def get_token_to_topic_type() -> dict[str, EnumTopicType]:
@@
-    return _get_token_mappings()[0]
+    return dict(_get_token_mappings()[0])

 def get_topic_type_to_token() -> dict[EnumTopicType, str]:
@@
-    return _get_token_mappings()[1]
+    return dict(_get_token_mappings()[1])
src/omnibase_core/models/contracts/subcontracts/model_event_bus_subcontract.py (1)

214-223: Validate topic metadata keys against declared suffixes.
publish_topic_metadata / subscribe_topic_metadata accept arbitrary keys today; typos or invalid suffixes will pass and can silently desync metadata from the declared topics. Consider validating keys against the suffix rules and (optionally) that they appear in the corresponding topic list.

Suggested validation hook (Pydantic v2)
-from pydantic import BaseModel, ConfigDict, Field, field_validator, model_validator
+from pydantic import BaseModel, ConfigDict, Field, ValidationInfo, field_validator, model_validator
@@
     subscribe_topic_metadata: dict[str, ModelTopicMeta] | None = Field(
         default=None,
         description="Optional metadata per subscribe topic suffix (keyed by suffix string)",
     )
 
+    `@field_validator`("publish_topic_metadata", "subscribe_topic_metadata", mode="after")
+    `@classmethod`
+    def validate_topic_metadata(
+        cls,
+        metadata: dict[str, ModelTopicMeta] | None,
+        info: ValidationInfo,
+    ) -> dict[str, ModelTopicMeta] | None:
+        if metadata is None:
+            return metadata
+
+        from omnibase_core.validation import validate_topic_suffix
+
+        if info.field_name == "publish_topic_metadata":
+            declared_topics = set(info.data.get("publish_topics", []))
+        else:
+            declared_topics = set(info.data.get("subscribe_topics", []))
+
+        for key in metadata:
+            result = validate_topic_suffix(key)
+            if not result.is_valid:
+                raise ValueError(f"Invalid topic metadata key '{key}': {result.error}")
+            if declared_topics and key not in declared_topics:
+                raise ValueError(
+                    f"Topic metadata key '{key}' is not declared in {info.field_name.replace('_metadata', '')}"
+                )
+        return metadata
src/omnibase_core/models/validation/model_topic_validation_result.py (1)

48-161: Enforce validation-result invariants in-code.
The invariants are documented but not enforced, so inconsistent states can be created (e.g., is_valid=True with parsed=None). A small model_validator would lock this down.

Suggested invariant check
-from pydantic import BaseModel, ConfigDict, Field
+from pydantic import BaseModel, ConfigDict, Field, model_validator
@@
 class ModelTopicValidationResult(BaseModel):
@@
     parsed: ModelTopicSuffixParts | None = Field(
         default=None,
         description="Parsed suffix parts if validation succeeded, None otherwise",
     )
+
+    `@model_validator`(mode="after")
+    def validate_invariants(self) -> "ModelTopicValidationResult":
+        if self.is_valid:
+            if self.parsed is None or self.error is not None:
+                raise ValueError(
+                    "is_valid=True requires parsed to be set and error to be None"
+                )
+        else:
+            if self.error is None or self.parsed is not None:
+                raise ValueError(
+                    "is_valid=False requires error to be set and parsed to be None"
+                )
+        return self

Comment on lines +62 to +70
token_to_type: dict[str, EnumTopicType] = {
"cmd": EnumTopicType.COMMANDS,
"dlq": EnumTopicType.DLQ,
"evt": EnumTopicType.EVENTS,
"intent": EnumTopicType.INTENTS,
"snapshot": EnumTopicType.SNAPSHOTS,
}
type_to_token: dict[EnumTopicType, str] = {v: k for k, v in token_to_type.items()}
valid_kinds: frozenset[str] = frozenset(token_to_type.keys())

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# First, find and examine the constants file mentioned
cat -n src/omnibase_core/constants/constants_topic_taxonomy.py | head -100

Repository: OmniNode-ai/omnibase_core

Length of output: 3705


🏁 Script executed:

# Search for validator constants and rules
rg -n "TOPIC_KIND_|VALID_TOPIC_KINDS" src/omnibase_core --type py

Repository: OmniNode-ai/omnibase_core

Length of output: 2183


🏁 Script executed:

# Search for validator suffix rules and kind validation
rg -n "kind not in|cmd.*evt.*dlq|snapshot" src/omnibase_core/validation/ --type py -A 3

Repository: OmniNode-ai/omnibase_core

Length of output: 2336


🏁 Script executed:

# Find ModelTopicSuffixParts definition
rg -n "class ModelTopicSuffixParts|EnumTopicType" src/omnibase_core/models/ --type py -B 2 -A 8

Repository: OmniNode-ai/omnibase_core

Length of output: 21823


Remove the "intent" token from token_to_type mapping.

The token_to_type dictionary includes "intent", but the validator and ModelTopicSuffixParts only allow cmd, evt, dlq, and snapshot as valid topic suffix tokens. The validator regex explicitly restricts kind to (cmd|evt|dlq|snapshot), and the centralized VALID_TOPIC_KINDS in model_topic_suffix_parts.py omits "intent". This mismatch means get_valid_topic_suffix_kinds() will incorrectly include "intent" even though it fails validation. Remove "intent" from the token map to align with the validator's canonical definition.

Proposed fix
    token_to_type: dict[str, EnumTopicType] = {
        "cmd": EnumTopicType.COMMANDS,
        "dlq": EnumTopicType.DLQ,
        "evt": EnumTopicType.EVENTS,
-       "intent": EnumTopicType.INTENTS,
        "snapshot": EnumTopicType.SNAPSHOTS,
    }
🤖 Prompt for AI Agents
In `@src/omnibase_core/constants/constants_topic_taxonomy.py` around lines 62 -
70, Remove the inconsistent "intent" entry from the token_to_type mapping so the
canonical topic-kind set matches the validator and ModelTopicSuffixParts; update
token_to_type (the dict mapping token_to_type) to only include "cmd", "dlq",
"evt", and "snapshot", regenerate type_to_token (which is built from
token_to_type) and valid_kinds so they reflect the same four tokens, ensuring
EnumTopicType values referenced remain consistent and
get_valid_topic_suffix_kinds() no longer exposes "intent".

Comment on lines +225 to +236
@field_validator("publish_topics", "subscribe_topics", mode="after")
@classmethod
def validate_topic_suffixes(cls, topics: list[str]) -> list[str]:
"""Validate each topic suffix against ONEX naming convention."""
# Import here to avoid circular import at module load time
from omnibase_core.validation import validate_topic_suffix

for topic in topics:
result = validate_topic_suffix(topic)
if not result.is_valid:
raise ValueError(f"Invalid topic suffix '{topic}': {result.error}")
return topics

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Use ModelOnexError for topic suffix validation failures.
This is ONEX-specific contract validation; using ValueError makes error handling inconsistent with the rest of this model. Consider raising ModelOnexError with EnumCoreErrorCode.VALIDATION_ERROR and context. As per coding guidelines, ONEX-specific validation errors should use ModelOnexError.

Example adjustment
         for topic in topics:
             result = validate_topic_suffix(topic)
             if not result.is_valid:
-                raise ValueError(f"Invalid topic suffix '{topic}': {result.error}")
+                raise ModelOnexError(
+                    message=f"Invalid topic suffix '{topic}': {result.error}",
+                    error_code=EnumCoreErrorCode.VALIDATION_ERROR,
+                    details=ModelErrorContext.with_context(
+                        {
+                            "error_type": ModelSchemaValue.from_value("valueerror"),
+                            "validation_context": ModelSchemaValue.from_value(
+                                "topic_suffix_validation"
+                            ),
+                            "topic": ModelSchemaValue.from_value(topic),
+                        },
+                    ),
+                )
         return topics
🤖 Prompt for AI Agents
In
`@src/omnibase_core/models/contracts/subcontracts/model_event_bus_subcontract.py`
around lines 225 - 236, Replace the ValueError raise in the
validate_topic_suffixes class validator with a ModelOnexError using
EnumCoreErrorCode.VALIDATION_ERROR: import ModelOnexError and EnumCoreErrorCode
at top of the function (or module) and when validate_topic_suffix(topic) returns
not is_valid raise ModelOnexError(EnumCoreErrorCode.VALIDATION_ERROR,
context={"field":"publish_topics/subscribe_topics","invalid_value":topic,"detail":result.error})
so ONEX-specific consumers can handle the validation failure consistently; keep
the loop and return topics as-is.

Comment on lines +1 to +37
"""
Parsed topic suffix parts model for ONEX topic naming validation.

This module provides ModelTopicSuffixParts, an immutable Pydantic model that
represents the parsed components of a valid ONEX topic suffix.

Topic Suffix Format:
onex.{kind}.{producer}.{event-name}.v{n}

Where:
- kind: Topic type token (cmd, evt, dlq, snapshot)
- producer: Service/producer name in kebab-case
- event-name: Event name in kebab-case
- v{n}: Version number (e.g., v1, v2)

Example:
>>> from omnibase_core.models.validation.model_topic_suffix_parts import (
... ModelTopicSuffixParts,
... )
>>> parts = ModelTopicSuffixParts(
... kind="evt",
... producer="omnimemory",
... event_name="intent-stored",
... version=1,
... raw_suffix="onex.evt.omnimemory.intent-stored.v1",
... )
>>> parts.kind
'evt'

Thread Safety:
ModelTopicSuffixParts is immutable (frozen=True) and thread-safe.
Instances can be safely shared across threads.

See Also:
- validator_topic_suffix: Validation utilities for topic suffixes
- ModelTopicValidationResult: Result model for suffix validation
"""

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Keep the module docstring concise.
This file is a simple data model; please trim the module docstring to ~3–6 lines and move extended examples/notes into the class docstring or docs. As per coding guidelines, please keep simple module docstrings concise.

🤖 Prompt for AI Agents
In `@src/omnibase_core/models/validation/model_topic_suffix_parts.py` around lines
1 - 37, The module docstring is too long; shorten it to a concise 3–6 line
summary describing the module purpose (parsed ONEX topic suffix parts) and the
topic format, then move the extended example, thread-safety note, and "See Also"
details into the ModelTopicSuffixParts class docstring (or external docs) —
update references to ModelTopicSuffixParts, validator_topic_suffix, and
ModelTopicValidationResult to ensure they remain documented in the class
docstring or docs.

Comment on lines +1 to +34
"""
Topic suffix validation result model for ONEX naming convention.

This module provides ModelTopicValidationResult, a Pydantic model that
encapsulates the result of validating an ONEX topic suffix.

The result includes:
- Validation status (is_valid)
- The original suffix that was validated
- Error message if validation failed
- Parsed suffix parts if validation succeeded

Example:
>>> from omnibase_core.models.validation.model_topic_validation_result import (
... ModelTopicValidationResult,
... )
>>> # Valid result
>>> result = ModelTopicValidationResult(
... is_valid=True,
... suffix="onex.evt.omnimemory.intent-stored.v1",
... error=None,
... parsed=some_parsed_parts,
... )
>>> result.is_valid
True

Thread Safety:
ModelTopicValidationResult is immutable (frozen=True) and thread-safe.
Instances can be safely shared across threads.

See Also:
- validator_topic_suffix: Validation utilities that produce this result
- ModelTopicSuffixParts: The parsed parts model
"""

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Trim the module docstring for a simple model.
Please condense the module docstring to ~3–6 lines and move examples to the class docstring or docs. As per coding guidelines, simple module docstrings should remain concise.

🤖 Prompt for AI Agents
In `@src/omnibase_core/models/validation/model_topic_validation_result.py` around
lines 1 - 34, The module docstring is overly long for this simple model; shorten
it to a concise 3–6 line summary and move the detailed examples and extended
notes into the ModelTopicValidationResult class docstring or external docs. Edit
the top-level docstring in the file to a brief one- or two-sentence description
of the module's purpose (e.g., "Pydantic model for ONEX topic suffix validation
results"), and relocate the example usage, thread-safety note, and "See Also"
references into the ModelTopicValidationResult class docstring (or project docs)
while keeping references to validator_topic_suffix and ModelTopicSuffixParts
there for discoverability.

Comment thread src/omnibase_core/validation/validator_topic_suffix.py Outdated
Comment thread tests/unit/validation/test_validator_topic_suffix.py
… [OMN-1537]

- Replace lenient pattern with strict pattern that rejects:
  - Trailing hyphens (e.g., "service-")
  - Consecutive hyphens (e.g., "my--service")
- Rename _LENIENT_KEBAB_PATTERN to _INTERNAL_KEBAB_PATTERN
- Improve pattern documentation with detailed rules
- Update tests to expect rejection of invalid kebab-case

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@src/omnibase_core/validation/validator_topic_suffix.py`:
- Around line 66-75: TOPIC_SUFFIX_PATTERN currently allows trailing or
consecutive hyphens; update it to match the stricter kebab-case used by
_INTERNAL_KEBAB_PATTERN by requiring segments separated by single hyphens where
each segment after the first contains one or more [a-z0-9] characters (i.e., use
a subpattern like [a-z][a-z0-9]*(?:-[a-z0-9]+)* for both the producer and
event-name parts) while keeping the existing prefix
(onex.(cmd|evt|dlq|intent|snapshot)) and version group (v(\d+)).
♻️ Duplicate comments (2)
src/omnibase_core/validation/validator_topic_suffix.py (1)

270-279: Resolve normalized vs original suffix in success results.

ModelTopicValidationResult.suffix is documented as the original suffix, but success currently stores the normalized lowercase value (while failures keep the original). Consider returning the stripped input or update the model docs to make normalization explicit.

🐛 Proposed fix (preserve trimmed original)
-    return ModelTopicValidationResult.success(suffix=normalized, parsed=parsed)
+    return ModelTopicValidationResult.success(suffix=stripped, parsed=parsed)
tests/unit/validation/test_validator_topic_suffix.py (1)

686-699: Add ticketed justifications for the # type: ignore[...] comments.

Each ignore needs a short explanation with a Linear ticket reference.

✏️ Suggested annotations
         with pytest.raises(Exception):  # ValidationError for frozen model
-            parts.kind = "cmd"  # type: ignore[misc]
+            # OMN-1537: mutation is intentional for immutability test.
+            parts.kind = "cmd"  # type: ignore[misc]
@@
             ModelTopicSuffixParts(
                 kind="evt",
                 producer="service",
                 event_name="event",
                 version=1,
                 raw_suffix="onex.evt.service.event.v1",
-                extra_field="not allowed",  # type: ignore[call-arg]
+                # OMN-1537: extra field is intentional to assert forbid-extra behavior.
+                extra_field="not allowed",  # type: ignore[call-arg]
             )
@@
-            ModelTopicSuffixParts(  # type: ignore[call-arg]
+            # OMN-1537: missing fields are intentional to assert required-field errors.
+            ModelTopicSuffixParts(  # type: ignore[call-arg]
                 kind="evt",
                 producer="service",
                 # missing event_name, version, raw_suffix
             )
@@
         result = ModelTopicValidationResult.failure(suffix="bad", error="error")
         with pytest.raises(Exception):  # ValidationError for frozen model
-            result.is_valid = True  # type: ignore[misc]
+            # OMN-1537: mutation is intentional for immutability test.
+            result.is_valid = True  # type: ignore[misc]
As per coding guidelines, every type ignore must include a ticketed justification.

Also applies to: 712-716, 758-762

🧹 Nitpick comments (2)
src/omnibase_core/validation/validator_topic_suffix.py (1)

1-44: Shorten the module docstring and add from __future__ import annotations.

The header docstring exceeds the 3–6 line guideline for simple modules, and the file is missing the required future-annotations import immediately after the docstring.

✏️ Add future-annotations import
+from __future__ import annotations
+
 import re
 from typing import Final
As per coding guidelines, please keep module docstrings concise and place the future import immediately after them.
tests/unit/validation/test_validator_topic_suffix.py (1)

1-18: Keep the test module docstring concise and add from __future__ import annotations.

The module docstring is longer than the 3–6 line guideline and the future-annotations import is missing after the docstring.

✏️ Add future-annotations import
+from __future__ import annotations
+
 import re
As per coding guidelines, please keep module docstrings concise and place the future import immediately after them.

Comment thread src/omnibase_core/validation/validator_topic_suffix.py
Claude (AI Assistant) added 5 commits January 26, 2026 07:43
…tion [OMN-1537]

- Return defensive copies from get_token_to_topic_type() and
  get_topic_type_to_token() to prevent mutable cache exposure
- Use stripped (trimmed original) instead of normalized (lowercased)
  suffix in validation success result for consistency with docs
- Add from __future__ import annotations to validator_topic_suffix.py
- Add model_validator to ModelTopicValidationResult to enforce
  is_valid/parsed/error invariants at construction time
- Add NOTE(OMN-1537) ticket references to type ignore comments in tests
… [OMN-1537]

- Use functools.lru_cache for thread-safe token mapping caching
- Consolidate duplicate kebab-case patterns into single KEBAB_CASE_PATTERN
- Add _CONTROL_CHARS constant for maintainability
- Enhance _rebuild_model() docstring with pytest-xdist context
…hods [OMN-1537]

Add missing DLQ (Dead Letter Queue) support to topic models after adding
EnumTopicType.DLQ in the topic suffix validation PR:

- Add dlq_default() factory method to ModelTopicConfig with 30-day retention
- Update registration_domain() and create_standard_manifest() to include DLQ
- Update test expectations from 4 to 5 topic types throughout test file
- Add test_dlq_default() test for new factory method

This fixes CI failures where tests expected all EnumTopicType values to be
present in standard manifests, but DLQ was missing from factory methods.
…N-1537]

- Add missing __all__ export list to ModelTopicMeta for API consistency
- Clarify raw_suffix field documentation: it stores canonical normalized
  form, not original input
- Add TestKindsSynchronization tests to ensure VALID_TOPIC_KINDS and
  TOPIC_SUFFIX_PATTERN regex stay in sync
…tion [OMN-1537]

Remove uppercase normalization - topic suffixes must now be lowercase.

Changes:
- Remove ~50 lines of uppercase_mode normalization logic
- Add strict lowercase validation at start of validate_topic_suffix()
- Reject any non-lowercase input with clear error message
- Update tests to expect rejection of uppercase/mixed-case input

Behavior change:
- Before: ONEX.EVT.SERVICE.EVENT.V1 → normalized to lowercase, accepted
- After: ONEX.EVT.SERVICE.EVENT.V1 → rejected (must be lowercase)

This enforces consistency with ONEX standards where all topics are lowercase.
@jonahgabriel
jonahgabriel merged commit c819932 into main Jan 26, 2026
30 checks passed
@jonahgabriel
jonahgabriel deleted the jonah/omn-1537-update-kafka-topic-constants-to-onex-naming-convention branch January 26, 2026 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant