⚠️ This comment is AI-generated (Claude Sonnet 4.6 via Claude Code) on behalf of tobixen
The FeatureSet class in caldav/compatibility_hints.py was written with a more ambitious vision than what ended up being needed. A review of all its methods found several items that can be simplified or removed. Details are documented in docs/design/TODO_COMPATIBILITY_HINTS.md.
Summary of findings
1. Dead/buggy bool branch in _convert_node
The else branch for non-server-feature types has inverted logic (not node.get('enable') returns True when enable=False) and is unreachable in practice — no caller uses is_supported(bool) on client-feature or server-peculiarity features. Should be replaced with an AssertionError to surface accidental misuse.
2. Possibly redundant _derive_from_subfeatures call in is_supported
For features with no dots, the method is called twice in the same lookup path (once in the loop body, once in the post-loop block). Needs careful review to confirm if one call is truly redundant.
3. _old_flags migration (ongoing)
Already marked ## TODO: remove this when it can be removed. ~12 server config dicts still carry old_flags lists. Each needs to be translated into new-style dotted features, then the shim code can be deleted.
4. feature_tree / _dots_to_tree can be simplified
Builds and caches a full nested-dict tree only to answer one-level subfeature lookups in find_feature. Can be replaced with a one-liner list comprehension, eliminating both methods.
5. _collapse_key carries vestigial enable/observed fields
These slots exist for the complex type vision but collapse is only ever called on server-feature nodes. Simplify to just return feature_dict.get('support').
6. copyFeatureSet should be renamed copy_feature_set
Inconsistent camelCase already noted with a TODO comment. No external callers; safe rename.
Suggested order
- Items 3 and 6 are independent and safe to do first.
- Items 1, 2, 4, 5 are interrelated around the type-system simplification and are best tackled together.
⚠️ This comment is AI-generated (Claude Sonnet 4.6 via Claude Code) on behalf of tobixen
The
FeatureSetclass incaldav/compatibility_hints.pywas written with a more ambitious vision than what ended up being needed. A review of all its methods found several items that can be simplified or removed. Details are documented indocs/design/TODO_COMPATIBILITY_HINTS.md.Summary of findings
1. Dead/buggy bool branch in
_convert_nodeThe
elsebranch for non-server-featuretypes has inverted logic (not node.get('enable')returnsTruewhenenable=False) and is unreachable in practice — no caller usesis_supported(bool)onclient-featureorserver-peculiarityfeatures. Should be replaced with anAssertionErrorto surface accidental misuse.2. Possibly redundant
_derive_from_subfeaturescall inis_supportedFor features with no dots, the method is called twice in the same lookup path (once in the loop body, once in the post-loop block). Needs careful review to confirm if one call is truly redundant.
3.
_old_flagsmigration (ongoing)Already marked
## TODO: remove this when it can be removed. ~12 server config dicts still carryold_flagslists. Each needs to be translated into new-style dotted features, then the shim code can be deleted.4.
feature_tree/_dots_to_treecan be simplifiedBuilds and caches a full nested-dict tree only to answer one-level subfeature lookups in
find_feature. Can be replaced with a one-liner list comprehension, eliminating both methods.5.
_collapse_keycarries vestigialenable/observedfieldsThese slots exist for the complex type vision but collapse is only ever called on
server-featurenodes. Simplify to justreturn feature_dict.get('support').6.
copyFeatureSetshould be renamedcopy_feature_setInconsistent camelCase already noted with a TODO comment. No external callers; safe rename.
Suggested order