tidaldb/.sdlc/features/m10-community-policy-engine/review.md

Code Review: Community Policy Engine (m10-community-policy-engine)

Summary

The implementation delivers all 6 tasks: schema-level CommunityPolicy types, SchemaBuilder integration with validation, CommunityPolicyEvaluator for write enforcement, read suppression threading through the retrieval pipeline, integration tests for all 10 spec scenarios, and a clean full-suite pass with fmt/clippy.

Correctness

Write Enforcement (T1–T3)

• CommunityPolicy struct correctly models allowed_write_signals, denied_write_signals, allowed_read_signals, denied_read_signals — all four lists are present and semantically distinct.
• CommunityPolicyEvaluator::check_write applies the correct priority: deny list checked first, then allow list enforcement when non-empty, then pass-through when both lists are empty (admin case). Logic matches spec.
• SchemaBuilder::community_policy validation correctly rejects: duplicate role names, unknown signal names in any policy list, allow/deny conflicts within the same list type. All three validated at build() time before the schema is sealed.
• signal_with_community_policy correctly routes through the evaluator and delegates to the signal write path on success. Error variants (PolicyViolationKind::CommunityWriteDenied, CommunityWriteNotAllowed) are distinct and carry the signal name for diagnostics.

Read Suppression (T4)

• CommunityContext is carried through Retrieve → RetrieveExecutor::stage3_score → ProfileExecutor via with_suppressed_signals(HashSet<String>).
• The suppressed set is built correctly: denied_read_signals always suppressed; signals not in allowed_read_signals suppressed when the allow list is non-empty; empty allow list → no suppression (admin passthrough).
• ProfileExecutor::compute_raw_score filters boosts whose signal is in the suppressed set.
• ProfileExecutor::score_by_sort returns 0.0 for all sort modes whose primary signal is suppressed (Hot→view, Trending→view+share, MostViewed→view, MostLiked→like, MostFollowed→follow, CreatorEngagementRate→view+like, MostCommented→comment, MostShared→share, LiveViewerCount→viewer_count, Controversial→like+dislike). Complete coverage.
• is_suppressed is a fast-path O(1) lookup with an early exit when the suppressed set is empty — no overhead on requests without community context.

Unknown Role Error (T4)

• When query.community is set but the role is not in the schema, stage3_score returns QueryError::InvalidFilter with a message referencing the role name. This satisfies test I10.

Test Coverage (T5)

All 10 spec scenarios are covered:

• I1: Write allowed signal succeeds and is readable with read_decay_score(..., 0) using Timestamp::now().
• I2: Write of denied_write_signals signal returns policy violation error referencing the signal name.
• I3: Write of signal not in non-empty allowed_write_signals list returns policy violation.
• I4: Admin role (empty allow/deny lists) allows writes of all signals.
• I5: Retrieve with moderator context (pin denied) returns same result count as unconstrained retrieve — suppression affects scoring, not candidate count.
• I6: Retrieve without community context does not suppress any signals.
• I7: Duplicate community policy name rejected at schema build time.
• I8: Unknown signal name in policy list rejected at schema build time.
• I9: Signal in both allow and deny list rejected at schema build time.
• I10: Write with unknown role name returns error referencing the role.

Code Health

• cargo clippy -- -D warnings: clean.
• cargo fmt -- --check: clean (revocation/tests.rs line-length formatting fixed).
• Duplicate module conflict (entities/revocation.rs vs entities/revocation/mod.rs) resolved by removing the stale flat file; the directory-form module is the canonical implementation.
• ProfileExecutor::new correctly changed from const fn to fn to support HashSet::new() initialization.
• 1299 lib tests passing; 10/10 M10 integration tests passing; m5_uat (9), m6_uat (9), m7_uat (10), m8_uat (8) all green.

Issues Found

None. All spec requirements are implemented, validated, and tested.

Verdict

Approved. The implementation is complete, correct, and production-ready.