[HNT-1289] Enable Custom Sections globally (#1179)

* [HNT-1289] Enable Custom Sections globally

* Fix lint and tests errors

* Clean up test code

* Fix lint errors

Author: mmiermans

merino/curated_recommendations/protocol.py

@@ -98,7 +98,6 @@ class ExperimentName(str, Enum):
     CONTEXTUAL_AD_V2_BETA_EXPERIMENT = "new-tab-contextual-ad-updates-v2-beta"
     CONTEXTUAL_AD_RELEASE_EXPERIMENT = "new-tab-contextual-ad-updates-release"
     CONTEXTUAL_AD_V2_RELEASE_EXPERIMENT = "new-tab-contextual-ad-updates-v2-release"
-    NEW_TAB_CUSTOM_SECTIONS_EXPERIMENT = "new-tab-custom-sections"
     # Experiment for doing local reranking of popular today via inferred interests
     INFERRED_LOCAL_EXPERIMENT = "new-tab-automated-personalization-local-ranking"
     INFERRED_LOCAL_EXPERIMENT_V2 = "new-tab-automated-personalization-local-ranking-2"

merino/curated_recommendations/sections.py

@@ -170,17 +170,15 @@ async def get_corpus_sections(
     min_feed_rank: int,
     include_subtopics: bool = False,
     scheduled_surface_backend: ScheduledSurfaceProtocol | None = None,
-    is_custom_sections_experiment: bool = False,
 ) -> tuple[Section | None, dict[str, Section]]:
-    """Fetch editorially curated sections with optional RSS vs. Zyte experiment filtering.
+    """Fetch curated sections.
 
     Args:
         sections_backend: Backend interface to fetch corpus sections.
         surface_id: Identifier for which surface to fetch sections.
         min_feed_rank: Starting rank offset for assigning receivedFeedRank.
         include_subtopics: Whether to include subtopic sections.
         scheduled_surface_backend: Backend interface to fetch scheduled corpus items (temporary)
-        is_custom_sections_experiment: Whether custom sections experiment is enabled.
 
     Returns:
         A tuple of headlines section (if present) & a mapping from section IDs to Section objects, each with a unique receivedFeedRank.
@@ -203,11 +201,10 @@ async def get_corpus_sections(
             is_legacy_section=False,
         )
 
-    # Apply RSS vs. Zyte experiment filtering and custom sections filtering
+    # Apply filtering based on subtopics experiment
     filtered_corpus_sections = filter_sections_by_experiment(
         remaining_raw_corpus_sections,
         include_subtopics,
-        is_custom_sections_experiment,
     )
 
     # Process the sections using the shared logic, passing the dict directly
@@ -310,13 +307,6 @@ def is_scheduler_holdback_experiment(request: CuratedRecommendationsRequest) ->
     )
 
 
-def is_custom_sections_experiment(request: CuratedRecommendationsRequest) -> bool:
-    """Return True if custom sections should be included based on experiments."""
-    return is_enrolled_in_experiment(
-        request, ExperimentName.NEW_TAB_CUSTOM_SECTIONS_EXPERIMENT.value, "treatment"
-    )
-
-
 def get_ranking_rescaler_for_branch(
     request: CuratedRecommendationsRequest,
 ) -> ExperimentRescaler | None:
@@ -351,17 +341,20 @@ def get_corpus_sections_for_legacy_topic(
 def filter_sections_by_experiment(
     corpus_sections: list[CorpusSection],
     include_subtopics: bool = False,
-    is_custom_sections_experiment: bool = False,
 ) -> dict[str, CorpusSection]:
-    """Filter sections based on RSS vs. Zyte experiment branch and custom sections experiment.
+    """Filter sections based on createSource and subtopics experiment.
+
+    Sections are included if they meet any of these criteria:
+    - Manually created sections (createSource == MANUAL)
+    - ML-generated legacy topic sections
+    - ML-generated subtopic sections (when subtopics experiment is enabled)
 
     Args:
         corpus_sections: List of CorpusSection objects
-        include_subtopics: Whether to include subtopic sections
-        is_custom_sections_experiment: Whether custom sections experiment is enabled
+        include_subtopics: Whether to include ML subtopic sections
 
     Returns:
-        Filtered sections
+        Dict mapping section IDs to CorpusSection objects
     """
     legacy_topics = get_legacy_topic_ids()
     result = {}
@@ -370,20 +363,9 @@ def filter_sections_by_experiment(
         section_id = section.externalId
         base_id = section_id
         is_legacy = base_id in legacy_topics
-        # is_legacy = base_id in legacy_topics
         is_manual_section = section.createSource == CreateSource.MANUAL
 
-        # Custom sections experiment: only include MANUAL sections in treatment, exclude them in control
-        if is_custom_sections_experiment:
-            # Treatment: only include MANUAL sections
-            if is_manual_section:
-                result[base_id] = section
-            continue
-
-        # Control/default: exclude MANUAL sections
-        if is_manual_section:
-            continue
-        if is_legacy or include_subtopics:
+        if is_manual_section or is_legacy or include_subtopics:
             result[base_id] = section
 
     return result
@@ -586,9 +568,6 @@ async def get_sections(
     # Determine if we should include subtopics based on experiments
     include_subtopics = is_subtopics_experiment(request)
 
-    # Determine if custom sections experiment is enabled
-    custom_sections_enabled = is_custom_sections_experiment(request)
-
     rescaler = get_ranking_rescaler_for_branch(request)
 
     headlines_corpus_section, corpus_sections_all = await get_corpus_sections(
@@ -597,7 +576,6 @@ async def get_sections(
         min_feed_rank=1,
         include_subtopics=include_subtopics,
         scheduled_surface_backend=scheduled_surface_backend,
-        is_custom_sections_experiment=custom_sections_enabled,
     )
 
     # Determine if we should include headlines section based on daily briefing experiment

tests/integration/api/v1/curated_recommendations/test_curated_recommendations.py

@@ -6,6 +6,7 @@
 import logging
 from typing import Any
 from unittest.mock import AsyncMock
+from uuid import UUID
 
 import aiodogstatsd
 from fastapi.testclient import TestClient
@@ -65,6 +66,18 @@
 from tests.types import FilterCaplogFixture
 
 
+def is_manual_section(section_id: str) -> bool:
+    """Check if section ID is a UUID (manually created sections use UUIDs, ML sections use human-readable IDs).
+
+    Note: This heuristic may become obsolete if all sections adopt UUID identifiers in the future.
+    """
+    try:
+        UUID(section_id)
+        return True
+    except ValueError:
+        return False
+
+
 class MockEngagementBackend(EngagementBackend):
     """Mock class implementing the protocol for EngagementBackend.
     experiment_traffic_fraction defines a fraction of traffic expected for an experiment
@@ -1344,11 +1357,12 @@ def test_sections_legacy_holdback(self, experiment_payload, client: TestClient):
         # Assert layouts are cycled
         assert_section_layouts_are_cycled(sections)
 
-        # The only sections are topic sections or "top_stories_section"
-        assert all(
-            section_name == "top_stories_section" or section_name in Topic
-            for section_name in sections
-        )
+        # Should have top_stories_section and legacy topic sections
+        # (may also have manually created sections)
+        assert "top_stories_section" in sections
+        legacy_topics = {topic.value for topic in Topic}
+        legacy_sections_present = [sid for sid in sections if sid in legacy_topics]
+        assert len(legacy_sections_present) > 0, "Should have at least some legacy topic sections"
 
     @pytest.mark.parametrize("locale", ["en-US", "de-DE"])
     @pytest.mark.parametrize(
@@ -1395,14 +1409,16 @@ def test_sections_feed_content(self, locale, experiment_payload, caplog, client:
             recs = section["recommendations"]
             assert {rec["receivedRank"] for rec in recs} == set(range(len(recs)))
 
-        # Check if non-ML experiment, only legacy sections returned
+        # Check section types based on experiment
         legacy_topics = {topic.value for topic in Topic}
 
         if experiment_payload.get("experimentName") != ExperimentName.ML_SECTIONS_EXPERIMENT.value:
-            # Non-ML sections experiment: All section keys (excluding top_stories) must be in legacy topics
+            # Non-ML sections experiment: Should have legacy topics and may have manually created sections
+            # but should not have ML subtopics
             for sid in sections:
-                if sid != "top_stories_section":
-                    assert sid in legacy_topics
+                if sid != "top_stories_section" and sid not in legacy_topics:
+                    # Non-legacy sections should only be manually created sections
+                    assert is_manual_section(sid), f"Unexpected section type: {sid}"
 
         # Check the recs used in top_stories_section are removed from their original ML sections.
         top_story_ids = {
@@ -1432,28 +1448,16 @@ def test_sections_feed_content(self, locale, experiment_payload, caplog, client:
                 == "Insider advice on where to eat, what to see, and how to enjoy the city like a local."
             )
 
-    @pytest.mark.parametrize(
-        "branch,should_have_manual,should_have_ml",
-        [
-            ("treatment", True, False),
-            ("control", False, True),
-        ],
-    )
-    def test_custom_sections_experiment(
-        self, branch: str, should_have_manual: bool, should_have_ml: bool, client: TestClient
-    ):
-        """Test custom sections experiment filters sections by createSource.
+    def test_sections_include_both_manual_and_ml(self, client: TestClient):
+        """Test that sections feed includes both manually created and ML-generated sections.
 
-        Treatment: Returns only MANUAL sections (createSource == "MANUAL")
-        Control: Excludes MANUAL sections (only ML sections returned)
+        Both MANUAL and ML sections should be returned together.
         """
         response = client.post(
             "/api/v1/curated-recommendations",
             json={
                 "locale": "en-US",
                 "feeds": ["sections"],
-                "experimentName": ExperimentName.NEW_TAB_CUSTOM_SECTIONS_EXPERIMENT.value,
-                "experimentBranch": branch,
             },
         )
         data = response.json()
@@ -1467,30 +1471,19 @@ def test_custom_sections_experiment(
         # top_stories_section should always be present
         assert "top_stories_section" in sections
 
-        manual_section_id = "d532b687-108a-4edb-a076-58a6945de714"
-
-        if should_have_manual:
-            # Treatment: Should NOT have ML sections
-            assert "music" not in sections
-            assert "nfl" not in sections
-            assert "tv" not in sections
-            assert "movies" not in sections
-            assert "nba" not in sections
-
-            # The MANUAL section "Tech stuff" may or may not appear depending on whether
-            # it has enough items after top stories are removed, but if it does appear,
-            # verify it has the correct title
-            if manual_section_id in sections:
-                assert sections[manual_section_id]["title"] == "Tech stuff"
-        else:
-            # Control: Should NOT have the MANUAL section
-            assert manual_section_id not in sections
-
-            # Should have ML sections (legacy topics only since not ML experiment)
-            legacy_topics = {topic.value for topic in Topic}
-            for sid in sections:
-                if sid != "top_stories_section":
-                    assert sid in legacy_topics
+        # Should have ML sections (legacy topics)
+        legacy_topics = {topic.value for topic in Topic}
+        ml_sections_found = [sid for sid in sections if sid in legacy_topics]
+        assert len(ml_sections_found) > 0, "Should have at least some ML legacy topic sections"
+
+        # Check if any manually created sections appear (they may or may not, depending on
+        # whether they have enough items after top stories are removed)
+        manual_sections = [sid for sid in sections if is_manual_section(sid)]
+        if manual_sections:
+            # If the "Tech stuff" manual section appears, verify it has the correct title
+            tech_stuff_id = "d532b687-108a-4edb-a076-58a6945de714"
+            if tech_stuff_id in sections:
+                assert sections[tech_stuff_id]["title"] == "Tech stuff"
 
     @pytest.mark.parametrize(
         "sections_payload",
@@ -1707,18 +1700,24 @@ def test_sections_filtering_by_region_and_holdback(
             and experiment_branch == "control"
         )
 
+        # Categorize non-legacy, non-top_stories sections
         non_legacy_section_ids = [
             sid
             for sid in sections
             if sid not in legacy_topics and sid not in {"top_stories_section"}
         ]
+        ml_subtopic_section_ids = [
+            sid for sid in non_legacy_section_ids if not is_manual_section(sid)
+        ]
 
         if expect_subtopics:
-            assert non_legacy_section_ids, "Expected subtopic sections for US treatment"
+            assert ml_subtopic_section_ids, "Expected ML subtopic sections for US treatment"
         else:
             assert (
-                not non_legacy_section_ids
-            ), f"Unexpected non-legacy sections: {non_legacy_section_ids}"
+                not ml_subtopic_section_ids
+            ), f"Unexpected ML subtopic sections: {ml_subtopic_section_ids}"
+
+        # Manually created sections may appear regardless of experiment settings
 
     def test_daily_briefing_experiment_headlines_section_returned(self, client: TestClient):
         """Test that the Headlines section is returned when the daily briefing experiment is enabled.