Repository navigation
[Feat]: Limit characters allowed in field/property names; reserve field/property names beginning with underscore #1383
Description
Activity
Hey @edwinyyyu, happy to take this. Plan: introduce
SafeFieldName = Annotated[str, AfterValidator(_is_valid_field_name)]incommon/api/spec.py(rules:starts with a-z, followed by [a-z0-9_]*, ≤32 chars) and apply it via PydanticAfterValidatorexclusively to input surfaces (MemoryMessage.metadata,*Spec.set_metadata,AddFeatureSpec.{tag,feature,category_name},UpdateFeatureSpec.{tag,feature,category_name,metadata}, CreateSemanticSetTypeSpec.metadata_tags).To avoid breaking reads of legacy records, response/entity models stay permissive, existing rows still deserialize, but new writes are forced to conform. Two architectural calls I want to flag:
-
set_idremains permissive (Out of scope). I initially considered tighteningset_idto stop the dirtyFeatureSet_<sanitized_set_id>Neo4j labels mentioned in [Bug][v2]: Neo4j node labels #633. However,set_idis used referentially; if we tighten it onAddFeatureSpecorUpdateFeatureSpec, users won't be able to add features to or update existing legacy sets that have non-conforming IDs. Fully resolving [Bug][v2]: Neo4j node labels #633 requires rethinking the identifier model, which belongs in a separate issue. The_sanitize_identifiertranslation layer will stay as a defensive backstop for now. -
Filter-expression key validation is out of scope.
SearchMemoriesSpec.filteris a free-form string referencing metadata keys and tightening those references needs a parser change which I believe belongs in a separate issue.
Push back if either call is wrong, otherwise I'll start working on the PR.
-
This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. If this issue is still relevant, please comment to keep it open, or add the
keep-openlabel if it should remain open indefinitely (e.g., a roadmap item awaiting a champion). Thank you for your contributions.Internally changing to use UUID would be better. Externally can allow a wider set.
Is your feature request related to a problem?
There are complaints about in-database representations being unreadable #633 . It makes debugging and auditing harder.
Many databases or storage solutions do not support arbitrary-length field or property names.
What may work on one backend one day may stop working on the next.
Describe the solution you'd like
The current approach (as in Neo4j) is to sanitize the names. This involves a lot of extra prefixing or bijective encoding and decoding steps that turns a simple field or property name into a long mess. It also results in incompatible data depending on how deeply wrapped the systems are.
Preferred
This will allow for easily readable and filterable metadata, compatible with (nearly?) all storage backends. No unbounded growth of metadata keys. At most one translation layer between database and source (like replacing underscores with hyphens; or the DB itself reserves underscore prefix, in which case we'd add a simple prefix).
Filters
All system-defined metadata (external/query-facing --> generic internal):
Describe alternatives you've considered
Each database supports different name limitations.
Additional context
No response