Owner Burhan

MCQ Module Cleanup — PRD

Context

The MCQ module already ships — the mcqs collection with full admin CRUD (av1), a mobile read/sync feed (v1), and web read/list (v2). Content writes are admin-only; v1/v2 are read-only consumers. The module is healthy: an audit (asym-context/.../mcq-module-audit.txt, 2026-06-22, re-verified against HEAD bedae188 on 2026-06-25) confirmed that 20 of 21 public MCQService methods and ~30 McqRepository methods are live with production route / cross-service / bg-task callers.

This PRD is a cleanup / slim-down delta, not a feature. It removes the small set of genuinely-dead symbols the audit surfaced, plus one fully-orphaned legacy bookmark service that is the only thing keeping a dead MCQ method alive. It does three things:

  1. Remove dead MCQService / McqRepository code — one zero-caller service method and three zero-caller repo methods.
  2. Remove the orphaned legacy v0_bookmark_services module — never imported, registered, or routed; replaced long ago by the live BookmarkService / bookmark_collections surface.
  3. Retire the write-orphaned like_count / dislike_count fields — denormalized counters whose only writer is dead, never read, never serialized to any client, and (verified) carrying no meaningful data in production.

It does not re-spec the MCQ feature, the solution/encryption model, the block/docket linkage mechanisms, the sort_order / reorder mechanism, or any of the MCQ-named lookalike modules — mcq_attrs (MCQAttributesService), mcq_docket_link (McqDocketLinkService), bookmark_mcq, user_mcq_daily_stats, and the content_rating MCQ router — all of which are separate entities with their own models/services.

Scope guardrail (decided). Every change here is internal: no client-facing request or response contract changes. like_count / dislike_count are removable within this guardrail precisely because they are not present in any response (see Data Model). Removing any field that is serialized would be out of scope.

Scope

MVP — a single-milestone, dead-code-only removal, delivered as one PR. Grouped below by layer for clarity, but there is no inter-group dependency and no sequencing within the milestone:

  1. Service-layer dead code. Remove MCQService.get_by_id (sole caller is the dead v0 module) and the entire orphaned OldBookmarkService / v0_bookmark_services.py module; trim one stale doc-comment.
  2. Repository-layer dead code. Remove three zero-caller McqRepository methods: count_by_years, bulk_update_solution_client, update_like_dislike_count.
  3. Dead aggregate fields. Remove like_count / dislike_count from McqDBModel and McqProjection.

M1 — MCQ dead-code removal (single PR)

Repo get_by_id is LIVE — do not remove (decided). Only the service method MCQService.get_by_id is dead. The repository method mcq_repository.get_by_id is heavily used (6 MCQService self-calls + flowchart / user-report services + exam / taxonomy bg-tasks) and is test-covered (src/tests/image_bank/test_image_bank_backfill.py:109). The two share a name; only the service one goes. This is the single most likely implementation mistake — call it out in the PR.

MCQService.get_by_id is dead via a dead caller (verified). Its only reference anywhere is services.mcq_service.get_by_id(...) at v0_bookmark_services.py:101, inside OldBookmarkService.create_mcq_bookmark. That method, that class, and that whole module are themselves dead (below), so MCQService.get_by_id is transitively dead. No test calls it.

The whole v0_bookmark_services module is dead (verified). v0_bookmark_services.py (OldBookmarkService, 14 methods, ~37 KB) is never imported in src/ (grep clean), not registered in unified_service_container.py, and no v0 bookmark router is mounted. Its 14 methods only reference each other. The single external trace is a stale doc-comment on the live BookmarkTimestampCursor class (api/v1/bookmark_collections/schemas.py:194: “…used by BookmarkService and v0_bookmark_services”). The live replacement is BookmarkService + the bookmark_collections surface.

BookmarkTimestampCursor stays (decided). It is shared with the live BookmarkService, so removing the v0 module must not remove the cursor — only trim the trailing “and v0_bookmark_services” from its docstring. Nothing else the v0 module imports becomes orphaned (all its imports — bookmark repos/projections, the cursor, the service container — are shared with live code).

Migrations kept as immutable history (decided). Two MCQ migrations are stale/broken but are retained as-is (mirrors the docket-cleanup convention):

  • jul_24_25/migrate_solution_admin_to_client_v2.py — calls mcq_repo.bulk_update_solution_client_v2 (a differently-named method that has never existed) and targets the retired solution_client_v2 field; it would AttributeError if ever run and is registered with no runner. Because it references a non-existent *_v2 name, removing bulk_update_solution_client does not affect it. Left in place.
  • feb_20_26/migrate_mcq_solution_to_client.py — Phase-1 $rename/$unset of obsolete versioned fields is stale; Phase-2 is still valid. Untouched.

like_count / dislike_count are dead, internal, and empty (decided). They are denormalized aggregate counters whose only writer is the dead update_like_dislike_count; they have zero readers anywhere; they are never copied into any response (response schemas are cherry-pick from_projection factories that omit them, and McqProjection is never returned by a route directly); and the owner has verified production carries no meaningful values in them. They are the half-built twin of bookmark_count, which did get wired (increment_bookmark_countBookmarkService ← bookmark bg-task, all live and retained). Removal is therefore a storage + internal-DTO change, not a response-contract change.

Removal manifest

GroupSymbol / field to removeLocationWhy dead
ServiceMCQService.get_by_idsrc/services/mcq_services.py:1386Sole caller is the dead v0_bookmark_services; no test.
Service moduleentire OldBookmarkService (14 methods)src/services/v0_bookmark_services.py (whole file)Never imported / registered / routed; only self-references + 1 stale doc-comment.
Doc-commenttrim “and v0_bookmark_services” from BookmarkTimestampCursor docstringsrc/api/v1/bookmark_collections/schemas.py:194The class stays (shared with live BookmarkService); only the dangling mention goes.
Repocount_by_yearssrc/repository/mcq_repository.py:2099Zero refs anywhere; the live year-count path is aggregate_mcq_counts_by_year_for_course.
Repobulk_update_solution_clientsrc/repository/mcq_repository.py:2678Zero live refs; the only mention is the broken jul_24_25 migration calling a non-existent *_v2 name.
Repoupdate_like_dislike_countsrc/repository/mcq_repository.py:2746Zero refs; sole writer of like_count/dislike_count.
Fieldlike_count, dislike_count (DB model)src/models/mcq_models/db_model.py:248-249 (+ schema example :639-640)Write-orphaned, read-inert, not serialized, no prod data.
Fieldlike_count, dislike_count (projection)src/models/mcq_models/projection_model.py:186-187 (+ schema example :312-313)Same; McqProjection is internal-only.
  • Test impact: 0 — no test references any removed symbol or field. (The get_by_id test hits in the suite target mcq_repository.get_by_id / exam_repository.get_by_id — different, live methods.)
  • Dead schemas: 0 — the v0 module defines only OldBookmarkService; every schema/projection/repo it imports is shared with live code. No response schema becomes orphaned.
  • Response-contract change: nonelike_count/dislike_count are absent from all av1/v1/v2 response schemas; McqProjection is never returned to a client.

Data Model (requirements-level)

McqDBModel loses two fields; everything else is unchanged.

FieldChangeNotes
like_countRemovedDenormalized “likes” counter. Only writer (update_like_dislike_count) is dead; zero readers; never serialized; no prod data.
dislike_countRemovedAs above.
bookmark_countunchangedThe genuinely-wired sibling counter — kept (increment_bookmark_countBookmarkService ← bookmark bg-task, all live).
(all other MCQ fields)unchangedOptions, solution, taxonomy/exam links, block linkage, status, version, etc. are untouched.

Existing Mongo data left in place (decided). Stored docs may physically carry like_count: 0 / dislike_count: 0 (Beanie wrote the default=0 on insert). Dropping the Pydantic fields makes Mongo ignore those values on read (schemaless). No hard delete now; an optional $unset cleanup migration may follow later (see Open Questions).

Delivery Milestones

#MilestoneOutcomeStatusPlan
1MCQ dead-code removalOne PR: MCQService.get_by_id + the whole v0_bookmark_services module gone; 3 dead McqRepository methods gone; like_count/dislike_count retired from model + projection. Scope expanded during spec review to also drop the 4 v0-only bookmark repositories + 4 v0 bookmark model packages (transitive orphans of the v0 deletion). No behaviour, request, or response change.✅ completespec · plan · PR #793

Plan key: BE = backend plan tasks in keystone (keystone/docs/superpowers/specs/ + …/plans/), authored from this PRD via /plan.

Sequencing. Single milestone, one PR. The three groups (service, repo, fields) are independent and need no internal ordering — but ship and review them as one diff since the total footprint is small (~1 method + 1 module + 3 methods + 2 fields). Migrations are not touched.

Open Questions

  • Optional $unset cleanup for like_count/dislike_count. Field drop leaves stored 0 values behind (harmless; ignored on read). Do we want a follow-up $unset migration to physically remove them, or leave them indefinitely? (Mirrors docket’s deferred-$unset decision; not required for this PR.) (Still open post-merge.)
  • Optional drop of the 4 orphaned v0 bookmark collections (bookmark_mcq, bookmark_docket, bookmark_mcq_bucket, bookmark_docket_bucket). Code + Beanie registration removed in PR #793; the Mongo collections remain. Drop later (manual or migration) once the removal has soaked, or leave indefinitely. (New, opened by the scope expansion; not required for this PR.)
  • Everything else is resolved during PRD review: dead-code only (no hygiene pass); migrations kept as immutable history; whole v0 module removed; repo get_by_id retained; BookmarkTimestampCursor retained; like_count/dislike_count removed as internal fields (verified no prod data, never serialized); bookmark_count retained.

Risks

RiskLikelihoodImpactMitigation
Removing the wrong get_by_id. Confusing the dead MCQService.get_by_id with the live, test-covered mcq_repository.get_by_id.MedHigh — breaks update/delete/get flows + cross-service callersExplicitly scoped: only the service method is removed. The repo method stays; the test suite (incl. test_image_bank_backfill.py) exercises it. Call out the distinction in the PR.
Hidden caller via dynamic dispatch. A removed symbol is reached through a path static grep missed (e.g. another services.mcq_service.* alias).LowHigh — runtime AttributeErrorAlias sweep already run (only services.mcq_service.get_by_id reached indirectly — its caller is the dead v0 module, also removed). Re-grep at implementation time; rely on the full test suite + a smoke pass.
v0 module removal orphaning a shared symbol. Removing v0_bookmark_services also drops something live code still needs (e.g. BookmarkTimestampCursor).LowMed — import error in BookmarkServiceVerified: the cursor and all other v0 imports are shared with the live BookmarkService; only the orphaned class + its self-references are removed, plus a docstring trim.
like_count/dislike_count turn out to be serialized somewhere. A client silently depends on the field.LowMed — response key disappearsVerified absent from every av1/v1/v2 response schema; McqProjection is never returned to a client directly. Re-confirm at implementation; no client coordination expected.
Stored field values cause confusion later. Left-behind like_count: 0 in Mongo.LowLow — cosmeticDocumented as schemaless leftovers; optional $unset migration tracked as an Open Question.

Technical Details (as built)

Status: ✅ built & merged (2026-06-25).

  • PR: #793 — refactor(mcq): remove verified dead code (MCQ + legacy v0 bookmark surface) — merged into dev on 2026-06-25.
  • Branch: chore/mcq-deadcode-cleanup (7 commits: 1 docs + 6 removal commits, one per spec group).
  • Spec: keystone/docs/superpowers/specs/2026-06-25-mcq-cleanup-deadcode-design.md
  • Plan: keystone/docs/superpowers/plans/2026-06-25-mcq-cleanup-deadcode.md (7 tasks; removal-style gate = grep-zero + import src.main smoke + ruff F401, since test impact was verified 0).
  • Built by: Burhan Kapdawala, via the PRD → spec → plan flow. Every removal was confirmed by an 8-way adversarial caller sweep + completeness critic before implementation.
  • Footprint: 17 files deleted (v0 service + 4 repos + 4 model packages = 12 files), 6 edited; ~5,270 lines removed. Implementation matched the spec exactly — no in-flight deviations from the approved spec.

Naming: PRD vs code

PRD termCode
MCQ entityMcqDBModel (collection mcqs) / McqProjection
MCQ service / repoMCQService (src/services/mcq_services.py), mcq_repository / mcq_repo alias (src/repository/mcq_repository.py)
Dead service methodMCQService.get_by_id (mcq_services.py:1386)
Live repo method (KEEP — do not confuse)mcq_repository.get_by_id (mcq_repository.py)
Orphaned legacy bookmark moduleOldBookmarkService (src/services/v0_bookmark_services.py)
Live bookmark replacement (KEEP)BookmarkService (src/services/bookmark_service.py) + bookmark_collections surface
Shared cursor (KEEP)BookmarkTimestampCursor (src/api/v1/bookmark_collections/schemas.py)
Dead repo methodscount_by_years, bulk_update_solution_client, update_like_dislike_count (mcq_repository.py:2099 / 2678 / 2746)
Live year-count path (KEEP)aggregate_mcq_counts_by_year_for_course (mcq_repository.py)
Dead aggregate fieldslike_count, dislike_count (db_model.py:248-249, projection_model.py:186-187)
Live counter sibling (KEEP)bookmark_count via increment_bookmark_count
Dead v0 bookmark repos (added to scope)bookmark_mcq_repository.py, bookmark_docket_repository.py, bookmark_mcq_bucket_repository.py, bookmark_docket_bucket_repository.py (src/repository/)
Dead v0 bookmark model packages (added to scope)bookmark_mcq_models/, bookmark_docket_models/, bookmark_mcq_bucket_models/, bookmark_docket_bucket_models/ (src/models/) — *DBModel + *Projection + create_*_unique_key
Beanie registration trimmed4 model entries removed from MODELS_TO_MIGRATE (src/db/mongodb/migrations.py)
Stale cursor type-hint fixedBookmarkTimestampCursor.create_next_cursor Union[…4 old projections…]List[BookmarkProjection] (bookmark_collections/schemas.py)
Live bookmark surfaces (KEEP)BookmarkDBModel / BookmarkProjection, BookmarkCollectionDBModel / BookmarkCollectionProjection, bookmark_repository, bookmark_collection_repository — distinct from the 4 removed v0 packages
Kept-as-history migrationsjul_24_25/migrate_solution_admin_to_client_v2.py, feb_20_26/migrate_mcq_solution_to_client.py

Decisions & deviations from the PRD (as built)

  • Scope expanded: 4 v0-only bookmark repositories removed (not in the original PRD manifest). Reason: pre-implementation verification found that deleting v0_bookmark_services left bookmark_mcq_repository, bookmark_docket_repository, bookmark_mcq_bucket_repository, and bookmark_docket_bucket_repository with zero callers — they were transitive orphans of the v0 deletion (same dead-via-dead-caller logic the PRD applied to MCQService.get_by_id). The live BookmarkService uses bookmark_repository / bookmark_collection_repository instead. Approved during spec review.
  • Scope expanded: 4 v0 bookmark model packages removed (bookmark_mcq_models/, bookmark_docket_models/, bookmark_mcq_bucket_models/, bookmark_docket_bucket_models/ — each db_model.py + projection_model.py + __init__.py, incl. the create_*_unique_key helpers). Reason: once the 4 repos went, the only remaining references to these models were (a) the Beanie registration list and (b) one stale type hint. The 4 projections were dead-via-stale-hint — BookmarkTimestampCursor.create_next_cursor declared a Union of them, but the live caller passes List[BookmarkProjection] (the projections weren’t even in the union). Approved during spec review.
  • Two consequential edits the expansion required: dropped 4 entries from MODELS_TO_MIGRATE in src/db/mongodb/migrations.py (Beanie de-registration only — collections untouched), and rewired the stale Union[…] hint to List[BookmarkProjection] in bookmark_collections/schemas.py (also dropped the now-unused Union import).
  • No deviation on the core PRD manifest. The service-vs-repo get_by_id distinction held (live mcq_repository.get_by_id retained); BookmarkTimestampCursor class retained (docstring trimmed only); bookmark_count / increment_bookmark_count retained; both stale MCQ migrations left as immutable history.
  • $unset for like_count/dislike_count deferred (decided) — see Open Questions / Migration.

Migration

  • Code-only PR — no data migration, no collection drop. Existing like_count/dislike_count values in stored mcqs docs are left in place (schemaless, ignored on read; MCQ models have no extra="forbid"). Optional $unset cleanup migration may follow once the field removal has soaked (see Open Questions).
  • 4 v0 bookmark collections left in Mongo. De-registering the 4 models from MODELS_TO_MIGRATE only stops Beanie from managing them — the bookmark_mcq, bookmark_docket, bookmark_mcq_bucket, and bookmark_docket_bucket collections persist untouched (decided: no migration to clear DB now; optional future drop tracked alongside the $unset follow-up).
  • The two stale MCQ migrations (jul_24_25, feb_20_26) are retained as immutable history and are not run or modified.