Add export_learner_data command for tenant off-boarding backup

[ZamanChaudhary]

Updated*

Ticket

EDLYPRODUCT-8334

export_tenant_reports_csv

Summary

export_tenant_reports_csv is a Django management command that generates a human-readable CSV export of a single tenant's data — grades, learner profiles, course enrollments, problem responses, and ORA2 (open response assessment) data — for use during tenant off-boarding.

It's intended for an operator (support, compliance, or the customer's own admin) who needs a complete, correctly-scoped snapshot of everything a tenant's learners did on the platform, in a format that's readable without any tooling — just CSV files and a manifest.

Why it exists

When a tenant leaves the platform, someone needs to hand them (or archive) their learner data before it's deleted. That data lives across five different report types the LMS already knows how to generate — but only for one course at a time, only through the instructor dashboard, and with no concept of "give me everything for this tenant, and only this tenant." This command automates that: run every relevant report for every course the tenant's members touch, filter it down to just their data, and package it into a single, audited export.

What it does, step by step

1. Resolves the tenant. Looks up the EdlySubOrganization for the given slug and its associated org(s).
2. Resolves membership. Finds every user with EdlyMultiSiteAccess for that tenant — this, not course/org matching, is what defines "belongs to this tenant."
3. Resolves the course set. Takes the union of (a) courses under the tenant's org(s) and (b) any course a real member is actually enrolled in, even if that course belongs to a different org. This catches cross-org enrollments that org-filtering alone would miss.
4. Runs the requested reports for each course in that set, by calling the LMS's own report-generator functions directly:
   • grades → CourseGradeReport (per-learner grade breakdown)
   • profiles → upload_students_csv (learner profile fields: name, email, language, location, etc.)
   • enrollments → derived from the same profile report (enrollment mode, course-varying fields)
   • problem_responses → ProblemResponses (every learner's answer to every problem)
   • ora2 → upload_ora2_data (open-response assessment submissions and scores)
   • may_enroll → upload_may_enroll_csv (opt-in only; not part of the default set)
5. Filters every report to tenant members only, before anything touches disk. A row that can't be confidently tied to a tenant member is dropped, not written.
6. Writes output:
   • Tenant-wide summaries: grades_summary.csv, learner_profile.csv, course_enrollments.csv
   • A per_course/ directory with one file per course per report type, for anyone who needs course-level detail
   • Error files (e.g. <course>__grades_errors.csv) for rows the underlying report itself flagged as failed
   • MANIFEST.json — the run's own audit record (see below)

How the report generation actually runs

The five report generators above are the exact same functions Studio's instructor dashboard uses — but normally they run as background Celery tasks, tracked by an InstructorTask database row, and their progress is polled from the UI.

This command doesn't do that. It patches the generators' task-context plumbing (runner._get_current_task and each module's upload_csv_to_report_store binding) so they run synchronously, in the same process as the management command itself, with no Celery task ever submitted. The upload calls that would normally push a CSV to the InstructorTask's report store are captured directly instead and routed into this command's own sinks.

Why not just submit real Celery tasks? Submitting five report tasks per course, for potentially hundreds of courses, would compete for space on a production instructor-task queue and risk stuck task reservations. Running everything in-process, one course at a time, keeps the whole export self-contained and predictable — at the cost of it being fully synchronous (see Limitations).

Data source: primary vs. replica

The command's own lookups — tenant resolution, membership, course-list resolution — are routed through a read replica (read_replica_or_default()) where one is configured, to keep this reporting workload off the primary database.

The five report generators themselves are not routed to the replica; they run against the primary as they normally would. This is deliberate: CourseGradeReport performs a read-then-write sequence internally that would be unsafe against replica lag (a write could be based on stale data). Routing the command's own queries to the replica while leaving the actual report logic untouched avoids introducing a new failure mode into code this command doesn't own.

Safety behavior

• --output-dir is required. There is no default export location — anywhere under the platform's public media root would be served by nginx, so the operator must consciously choose a private destination every run.
• Sensitive fields are opt-in. The raw meta profile JSON blob is excluded unless --include-fields explicitly asks for it (and confirms with --allow-meta-field). The may_enroll report, which can include people who never actually enrolled, is excluded from the default report set.
• CSV formula injection is escaped on every write path — any cell starting with =, +, -, or @ is prefixed to prevent it from being interpreted as a formula if the CSV is later opened in a spreadsheet app.
• Never write unattributable data. If a report's rows can't be reliably matched back to a specific tenant member — this comes up with ORA2's anonymized student IDs — that report is skipped for that course and the reason is recorded in the manifest, rather than risk writing another tenant's data into this export.
• problem_responses requires --as-user. That report needs an acting staff identity to resolve course-block access; a non-staff or missing identity is rejected up front instead of silently producing an incomplete export.

Interruption and failure handling

If the run is interrupted partway through (Ctrl-C, OOM-kill, an unexpected exception), the command still writes whatever manifest and CSV data it can before exiting, and marks the manifest status incomplete so it's never mistaken for a clean run. A course-level report failure doesn't stop the rest of the export — it's recorded against that course and the run continues, ending with a complete_with_errors status if anything failed. The manifest is the single source of truth for "did this actually work" — the process exit code and stdout banner both reflect it.

The manifest

MANIFEST.json, written into the output directory alongside the CSVs, records:

{
  "slug": "acme",
  "operator": "jane.doe",
  "generated_at": "...",
  "reports_requested": ["grades", "profiles", "enrollments"],
  "courses": {
    "course-v1:AcmeX+CS101+2026": {
      "grades": {"status": "success", "rows": 42},
      "profiles": {"status": "success", "rows": 42}
    }
  },
  "courses_completed": [...],
  "courses_with_errors": [...],
  "courses_needing_review": [...],
  "known_gaps": {...},
  "status": "complete"
}

status is one of complete, complete_with_errors, or incomplete — the one field to check before trusting an export is done.

Usage

python manage.py lms export_tenant_reports_csv <tenant-slug> \
    --output-dir /private/path/outside/media/root \
    --reports grades,profiles,enrollments,problem_responses,ora2 \
    --as-user <staff-username>

Flag	Required	Purpose
slug (positional)	yes	Tenant's EdlySubOrganization slug
--output-dir	yes	Where to write the export; no default
--reports	no	Comma-separated report types; defaults to grades, profiles, enrollments, problem_responses, ora2
--as-user	only if problem_responses requested	Staff identity to act as when resolving course-block access
--include-fields	no	Override the default profile field allowlist
--allow-meta-field	no	Confirms an intentional request for the raw meta field
--skip-course	no, repeatable	Exclude a specific course from the run
--ora2-identity-column	no	Manually specify the ORA2 identity column, bypassing runtime detection
--max-problem-responses	no	Override the platform's per-course response cap for this run only
--dry-run	no	Print the resolved course/report matrix; write nothing

Limitations

• Fully synchronous. For a tenant with many courses, the run takes proportionally longer than a queued background task would, since everything happens in one process, one course at a time.
• ORA2 rows can be skipped per-course when the identity column can't be reliably detected — a deliberate trade-off in favor of never leaking cross-tenant data, not a bug.
• Report generators run against the primary DB, not the replica, for the correctness reason described above — this adds to primary DB load during the run.

[Waleed-Mujahid]

Review Council Report: PR #756 — export_learner_data

Three-lens review ran (correctness, security, openedx) plus cross-reference against a proven production tenant backup SQL script (55 tables, used for an actual Edly client off-boarding).

Bottom line: Implementation is well-engineered, but TABLE_SCOPE is ~75% incomplete vs. what a production backup proved necessary for complete tenant data export. Requesting changes.

---

Blocker (must fix)

#	Finding
B1	Missing submissions/assessment chain (ORA data silently lost) — 12 tables omitted: submissions_studentitem, submissions_submission, submissions_score, assessment_assessment, assessment_assessmentfeedback, assessment_assessmentfeedback_assessments, assessment_assessmentfeedback_options, assessment_peerworkflow, assessment_peerworkflowitem, assessment_staffworkflow, assessment_studenttrainingworkflow, assessment_studenttrainingworkflowitem, problem_builder_answer. Scope chain through student_anonymoususerid → submissions_studentitem.student_id. The export command handles student_anonymoususerid already, so the join chain is buildable. Any tenant using ORA = all learner submissions, scores, peer+staff assessments non-restorable.
B2	Missing social_auth_usersocialauth (SSO login breaks on restore) — provider/uid/extra_data per user not exported. Restored users can't authenticate through social/SSO provider.
B3	Missing proctoring/verification tables — proctoring_proctoredexamstudentattempt, verify_student_softwaresecurephotoverification, verify_student_manualverification, verify_student_ssoverification omitted. Proctored exam attempt records = complete data loss.
B4	Missing edly_edlyuserprofile — is_blocked/is_social_user flags per user lost. After restore: blocked users unblocked, social-user flag reset.
B5	Missing student_courseaccessrole — course staff/instructor/CCX coach role assignments not exported. Tenant loses all course-level access control on restore.

Should-Fix

#	Finding
S1	15 user-scoped tables missing (all easy: table, user_id, needs_course_filter=False) — student_userattribute, user_api_userpreference, user_api_usercoursetag, external_user_ids_externalid, edly_twofactorbypass, courseware_xmodulestudentinfofield, courseware_xmodulestudentprefsfield, edly_studentcourseprogress, bookmarks_bookmark, course_goals_coursegoal, milestones_usermilestone, edx_when_userdate, lti_consumer_ltiagsscore, teams_courseteammembership, django_comment_client_role_users, courseware_studentfieldoverride. No new infrastructure needed.
S2	Figures analytics tables missing — figures_enrollmentdata, figures_learnercoursegrademetrics. Scope by user_id + site_id + course_id.
S3	kwl/journal xblock content missing — kwl_djangoapp_kwlmodel.user and journal_djangoapp_journalmodel.user are CHAR columns, not FK ints. Production backup SQL: WHERE user IN (SELECT DISTINCT CAST(user_id AS CHAR) COLLATE utf8mb4_unicode_ci FROM edly_edlymultisiteaccess WHERE sub_org_id = @suborg_id).
S4	Course scoping differs from production backup — production backup used course_id LIKE 'course-v1:%' (no org filter). PR uses CourseOverview.objects.filter(org__in=orgs). Cross-org enrollments silently dropped. This may be correct but is undocumented.
S5	grades_visibleblocks missing — visible-blocks JSON needed for grade reconstruction after restore.
S6	certificates_certificateinvalidation missing — cert invalidation records lost (FK through generated_certificate_id).
S7	User resolution fallback may leak cross-tenant data — when EdlyMultiSiteAccess is empty, fallback to CourseEnrollment by course_ids can pull users from other tenants (course IDs are platform-global). Production backup never used this fallback.

Nits

#	Finding
N1	student_manualenrollmentaudit included — table confirmed on Koa (has enrollment_id FK). Valid addition, just not in the original backup SQL.
N2	No data checksum in MANIFEST.json — bundle integrity not verifiable downstream.
N3	Default output dir under /tmp/edm_exports/ — auto-clean on many systems could delete exports. Recommend a more durable default.

---

Reference: Production Backup SQL (used for actual client off-boarding)

For context, the proven SQL dump that was used for an Edly tenant data extraction:

SET @lms_domain = CONVERT('tenant.example.com' USING utf8mb4) COLLATE utf8mb4_unicode_ci;
SET @suborg_id = (SELECT eso.id FROM edly_edlysuborganization eso JOIN django_site ds ON ds.id = eso.lms_site_id WHERE ds.domain = @lms_domain LIMIT 1);

-- 1. Core user tables (user_id scope, no course filter needed)
SELECT * FROM auth_user WHERE id IN (SELECT DISTINCT user_id FROM edly_edlymultisiteaccess WHERE sub_org_id = @suborg_id);
SELECT * FROM auth_userprofile WHERE user_id IN (SELECT DISTINCT user_id FROM edly_edlymultisiteaccess WHERE sub_org_id = @suborg_id);
SELECT * FROM auth_registration WHERE user_id IN (SELECT DISTINCT user_id FROM edly_edlymultisiteaccess WHERE sub_org_id = @suborg_id);
SELECT * FROM student_anonymoususerid WHERE user_id IN (SELECT DISTINCT user_id FROM edly_edlymultisiteaccess WHERE sub_org_id = @suborg_id);
SELECT * FROM social_auth_usersocialauth WHERE user_id IN (SELECT DISTINCT user_id FROM edly_edlymultisiteaccess WHERE sub_org_id = @suborg_id);
SELECT * FROM student_userattribute WHERE user_id IN (SELECT DISTINCT user_id FROM edly_edlymultisiteaccess WHERE sub_org_id = @suborg_id);
SELECT * FROM user_api_userpreference WHERE user_id IN (SELECT DISTINCT user_id FROM edly_edlymultisiteaccess WHERE sub_org_id = @suborg_id);
SELECT * FROM user_api_usercoursetag WHERE user_id IN (SELECT DISTINCT user_id FROM edly_edlymultisiteaccess WHERE sub_org_id = @suborg_id);
SELECT * FROM external_user_ids_externalid WHERE user_id IN (SELECT DISTINCT user_id FROM edly_edlymultisiteaccess WHERE sub_org_id = @suborg_id);
SELECT * FROM edly_edlyuserprofile WHERE user_id IN (SELECT DISTINCT user_id FROM edly_edlymultisiteaccess WHERE sub_org_id = @suborg_id);
SELECT * FROM edly_twofactorbypass WHERE user_id IN (SELECT DISTINCT user_id FROM edly_edlymultisiteaccess WHERE sub_org_id = @suborg_id);

-- 2. Enrollments & course access (user_id + course scope)
SELECT * FROM student_courseenrollment WHERE user_id IN (SELECT DISTINCT user_id FROM edly_edlymultisiteaccess WHERE sub_org_id = @suborg_id) AND course_id LIKE 'course-v1:%';
SELECT * FROM student_courseenrollmentattribute WHERE enrollment_id IN (SELECT id FROM student_courseenrollment WHERE user_id IN (SELECT DISTINCT user_id FROM edly_edlymultisiteaccess WHERE sub_org_id = @suborg_id) AND course_id LIKE 'course-v1:%');
SELECT * FROM student_courseaccessrole WHERE user_id IN (SELECT DISTINCT user_id FROM edly_edlymultisiteaccess WHERE sub_org_id = @suborg_id);

-- 3. Course activity & grades (student_id/user_id + course scope)
SELECT * FROM courseware_studentmodule WHERE student_id IN (SELECT DISTINCT user_id FROM edly_edlymultisiteaccess WHERE sub_org_id = @suborg_id);
SELECT * FROM completion_blockcompletion WHERE user_id IN (SELECT DISTINCT user_id FROM edly_edlymultisiteaccess WHERE sub_org_id = @suborg_id);
SELECT * FROM courseware_xmodulestudentinfofield WHERE student_id IN (SELECT DISTINCT user_id FROM edly_edlymultisiteaccess WHERE sub_org_id = @suborg_id);
SELECT * FROM courseware_xmodulestudentprefsfield WHERE student_id IN (SELECT DISTINCT user_id FROM edly_edlymultisiteaccess WHERE sub_org_id = @suborg_id);
SELECT * FROM grades_persistentcoursegrade WHERE user_id IN (SELECT DISTINCT user_id FROM edly_edlymultisiteaccess WHERE sub_org_id = @suborg_id);
SELECT * FROM grades_persistentsubsectiongrade WHERE user_id IN (SELECT DISTINCT user_id FROM edly_edlymultisiteaccess WHERE sub_org_id = @suborg_id);
SELECT * FROM grades_visibleblocks WHERE course_id IN (SELECT DISTINCT course_id FROM grades_persistentcoursegrade WHERE user_id IN (SELECT DISTINCT user_id FROM edly_edlymultisiteaccess WHERE sub_org_id = @suborg_id));

-- 4. Certificates
SELECT * FROM certificates_generatedcertificate WHERE user_id IN (SELECT DISTINCT user_id FROM edly_edlymultisiteaccess WHERE sub_org_id = @suborg_id);
SELECT * FROM certificates_certificateinvalidation WHERE generated_certificate_id IN (SELECT id FROM certificates_generatedcertificate WHERE user_id IN (SELECT DISTINCT user_id FROM edly_edlymultisiteaccess WHERE sub_org_id = @suborg_id));

-- 5. Submissions & ORA (anonymous_user_id chain)
SELECT * FROM submissions_studentitem WHERE student_id IN (SELECT anonymous_user_id FROM student_anonymoususerid WHERE user_id IN (SELECT DISTINCT user_id FROM edly_edlymultisiteaccess WHERE sub_org_id = @suborg_id));
SELECT * FROM submissions_submission WHERE student_item_id IN (SELECT id FROM submissions_studentitem WHERE student_id IN (SELECT anonymous_user_id FROM student_anonymoususerid WHERE user_id IN (SELECT DISTINCT user_id FROM edly_edlymultisiteaccess WHERE sub_org_id = @suborg_id)));
SELECT * FROM submissions_score WHERE student_item_id IN (SELECT id FROM submissions_studentitem ...);
SELECT * FROM assessment_assessment WHERE submission_uuid IN (SELECT uuid FROM submissions_submission WHERE student_item_id IN (SELECT id FROM submissions_studentitem ...));
SELECT * FROM assessment_assessmentfeedback WHERE submission_uuid IN (SELECT uuid FROM submissions_submission ...);
SELECT * FROM assessment_assessmentfeedback_assessments WHERE assessmentfeedback_id IN (SELECT id FROM assessment_assessmentfeedback WHERE submission_uuid IN (...));
SELECT * FROM assessment_assessmentfeedback_options WHERE assessmentfeedback_id IN (SELECT id FROM assessment_assessmentfeedback ...);
SELECT * FROM assessment_peerworkflow WHERE submission_uuid IN (SELECT uuid FROM submissions_submission ...);
SELECT * FROM assessment_peerworkflowitem WHERE author_id IN (SELECT id FROM assessment_peerworkflow WHERE submission_uuid IN (...));
SELECT * FROM assessment_staffworkflow WHERE submission_uuid IN (SELECT uuid FROM submissions_submission ...);
SELECT * FROM assessment_studenttrainingworkflow WHERE submission_uuid IN (SELECT uuid FROM submissions_submission ...);
SELECT * FROM assessment_studenttrainingworkflowitem WHERE workflow_id IN (SELECT id FROM assessment_studenttrainingworkflow WHERE submission_uuid IN (...));
SELECT * FROM problem_builder_answer WHERE student_id IN (SELECT anonymous_user_id FROM student_anonymoususerid WHERE user_id IN (SELECT DISTINCT user_id FROM edly_edlymultisiteaccess WHERE sub_org_id = @suborg_id));

-- 6. User extras
SELECT * FROM bookmarks_bookmark WHERE user_id IN (SELECT DISTINCT user_id FROM edly_edlymultisiteaccess WHERE sub_org_id = @suborg_id);
SELECT * FROM course_goals_coursegoal WHERE user_id IN (SELECT DISTINCT user_id FROM edly_edlymultisiteaccess WHERE sub_org_id = @suborg_id);
SELECT * FROM milestones_usermilestone WHERE user_id IN (SELECT DISTINCT user_id FROM edly_edlymultisiteaccess WHERE sub_org_id = @suborg_id);
SELECT * FROM edx_when_userdate WHERE user_id IN (SELECT DISTINCT user_id FROM edly_edlymultisiteaccess WHERE sub_org_id = @suborg_id);

-- 7. Verification & proctoring
SELECT * FROM verify_student_softwaresecurephotoverification WHERE user_id IN (SELECT DISTINCT user_id FROM edly_edlymultisiteaccess WHERE sub_org_id = @suborg_id);
SELECT * FROM verify_student_manualverification WHERE user_id IN (SELECT DISTINCT user_id FROM edly_edlymultisiteaccess WHERE sub_org_id = @suborg_id);
SELECT * FROM verify_student_ssoverification WHERE user_id IN (SELECT DISTINCT user_id FROM edly_edlymultisiteaccess WHERE sub_org_id = @suborg_id);
SELECT * FROM proctoring_proctoredexamstudentattempt WHERE user_id IN (SELECT DISTINCT user_id FROM edly_edlymultisiteaccess WHERE sub_org_id = @suborg_id);

-- 8. LTI, teams, forums
SELECT * FROM lti_consumer_ltiagsscore WHERE user_id IN (SELECT DISTINCT user_id FROM edly_edlymultisiteaccess WHERE sub_org_id = @suborg_id);
SELECT * FROM teams_courseteammembership WHERE user_id IN (SELECT DISTINCT user_id FROM edly_edlymultisiteaccess WHERE sub_org_id = @suborg_id);
SELECT * FROM django_comment_client_role_users WHERE user_id IN (SELECT DISTINCT user_id FROM edly_edlymultisiteaccess WHERE sub_org_id = @suborg_id);

-- 9. Figures analytics
SELECT * FROM figures_enrollmentdata WHERE user_id IN (SELECT DISTINCT user_id FROM edly_edlymultisiteaccess WHERE sub_org_id = @suborg_id);
SELECT * FROM figures_learnercoursegrademetrics WHERE user_id IN (SELECT DISTINCT user_id FROM edly_edlymultisiteaccess WHERE sub_org_id = @suborg_id);

-- 10. Xblock data (non-standard user column types)
SELECT * FROM kwl_djangoapp_kwlmodel WHERE user IN (SELECT DISTINCT CAST(user_id AS CHAR) COLLATE utf8mb4_0900_ai_ci FROM edly_edlymultisiteaccess WHERE sub_org_id = @suborg_id);
SELECT * FROM journal_djangoapp_journalmodel WHERE user IN (SELECT DISTINCT CAST(user_id AS CHAR) COLLATE utf8mb4_unicode_ci FROM edly_edlymultisiteaccess WHERE sub_org_id = @suborg_id);

-- 11. Progress & membership
SELECT * FROM edly_studentcourseprogress WHERE student_id IN (SELECT DISTINCT user_id FROM edly_edlymultisiteaccess WHERE sub_org_id = @suborg_id);
SELECT * FROM courseware_studentfieldoverride WHERE student_id IN (SELECT DISTINCT user_id FROM edly_edlymultisiteaccess WHERE sub_org_id = @suborg_id);
SELECT * FROM edly_edlymultisiteaccess WHERE sub_org_id = @suborg_id;

The PR currently handles 14 of these 55 tables. The ORA chain (12 tables), proctoring/verification (4 tables), social auth, course roles, user preferences/attrs, and xblock data (kwl/journal) are the critical gaps.

Requesting changes per blockers B1-B5 above.

[ZamanChaudhary]

Thanks for the thorough audit — the original export really did only cover a fraction of what a full learner backup needs. Here's what changed and what's still open.

Fully addressed

• Table scope: added the full ORA/submissions assessment chain (16 tables, traced through the real submissions/assessment app FK graph — student_item → submission → score/assessment/peerworkflow/staffworkflow/studenttrainingworkflow, etc.), proctoring + ID-verification tables, social_auth_usersocialauth, edly_edlyuserprofile, student_courseaccessrole, Figures analytics, and the remaining ~15 trivial user/student-scoped tables (S1).
• Scoping model (B5/S4/S7): switched to membership-only scoping. Dropped the CourseEnrollment-derived fallback so non-members can no longer leak into a tenant's export, and removed course-org filtering from course-scoped tables so a member's enrollments/grades/progress in courses outside the tenant's own orgs are no longer silently dropped.
• S3 (CHAR-column user matching): fixed for kwl_djangoapp_kwlmodel.user and (found independently) lti_consumer_ltiagsscore.user_id, both of which store a stringified id rather than an integer FK.
• S5/S6: grades_visibleblocks and certificates_certificateinvalidation added via their real FK chains (visible_blocks_hash, generated_certificate_id).
• N2/N3: manifest now carries a per-file sha256 checksum; default output dir moved off bare /tmp (tmp-reapers could purge a bundle before it's retrieved).
• Also caught and fixed two things not in your list: SoftwareSecurePhotoVerification needed separate handling as a Django multi-table-inheritance child of PhotoVerification (no user_id column of its own), and a submission_uuid format mismatch (raw hex vs. hyphenated string) that would have silently zeroed out assessment_assessmentfeedback and dropped a member's own graded-by-someone-else submissions from assessment_assessment/assessment_staffworkflow.

Deliberately not done, with reasons

• journal_djangoapp_journalmodel (S3) — journal-xblock isn't installed in this Koa checkout at all (checked requirements/edx/*.txt), so there's no table to export. Flagged as a gap in the module docstring, not silently dropped.
• problem_builder_answer (B1) — same reason, problem-builder isn't installed here.
• ORA rubric/definition tables (assessment_rubric, assessment_criterion, assessment_criterionoption, assessment_trainingexample, assessment_assessmentfeedbackoption) and the assessmentfeedback M2M through-tables — excluded as course-content-adjacent reference data rather than per-learner records, consistent with course content itself already being out of scope for this command. A restore assumes these already exist on the target.
• Proctoring history/allowance tables (proctoring_proctoredexamstudentattempthistory, proctoring_proctoredexamstudentallowance{,history}) beyond the one attempt table you named — not added, wasn't in scope per your review. Say the word if you want these folded in too.

Every added table/column was checked against this checkout's actual pinned dependency versions rather than assumed. Happy to adjust scope on any of the "deliberately not done" items if you'd rather have them in this PR than as a fast-follow.

[Waleed-Mujahid]

Adversarial review — PR #756 export_learner_data

Reviewed feat/export-learner-data @ 1d60ad9c (1038 lines command + 940 lines tests), read line by line, with claims verified against the Koa checkout rather than inferred. Cross-referenced against the production tenant-backup SQL from the earlier review round.

Requesting changes. Two independent problems:

1. The PR solves a different problem than the one we have. The confirmed requirement is an off-boarding client asking for their learners' progress and data, delivered as CSV for non-technical staff. This command is built as an internal Koa→Ulmo restore bundle emitting per-table JSON (its own docstring: "Output bundle shape matches the Ulmo-side import_learner_data command exactly"). Those two artifacts have opposite requirements, and building the second and handing it over creates both a usability problem and a data-protection problem (§1, §2).
2. The implementation is not safe to run against the production primary, which is where it's slated to run. One defect is a non-terminating loop that fills the disk (§3); one fails outright at real tenant scale (§4); the DB access pattern has no snapshot isolation and targets the writer (§5, §6).

The engineering care here is real — the streaming writer, the os.open mode handling, the MTI and submission_uuid findings the author caught independently are all good work. Flagging one thing about the docstrings specifically: they're unusually thorough, which makes it easy to trust them, and in at least two places (§3, §9) they confidently describe behaviour the code does not implement. That's worth calling out because it affected how this PR was reviewed the first time round.

---

§1 — BLOCKER: the bundle ships credentials and live third-party tokens to a departing client

Given the artifact goes to the client, the following are in it, in cleartext:

Column	What it is
auth_user.password	password hashes
auth_registration.activation_key	can be used to activate accounts
social_auth_usersocialauth.extra_data	OAuth access + refresh tokens, plaintext JSON
edly_twofactorbypass	2FA bypass records
verify_student_photoverification.photo_id_key	key protecting the learner's government-ID photo
auth_userprofile	full name, year of birth, gender, mailing address, phone

Handing a departing third party live OAuth refresh tokens for their learners' identity-provider accounts, plus password hashes, is not something we can do regardless of how well the export code is written. The file permissions work (0700/0600 created via os.open with an explicit mode, correctly avoiding the umask window — genuinely well done), but permissions protect the file on our disk, not the contents after handover.

Also absent: encryption at rest, any redaction mode, any --include-secrets gate, an audit record of who ran it / row counts / destination (stdout only), and a retention/destruction step.

Required: a client-facing export must never include password, activation_key, extra_data, photo_id_key, or edly_twofactorbypass. Default-deny, with an explicit flag if an internal-restore variant ever needs them.

§2 — BLOCKER: wrong output format. The client has asked for CSV, not JSON.

The client's stated requirement is CSV — their people are non-technical and need to open this in Excel or Sheets. This command emits JSON, and specifically a shape that is harder to consume than ordinary JSON would be.

Output is MANIFEST.json plus ~50 files at learners/<table>.json, each:

{"columns": ["id", "user_id", ...], "rows": [[1, 42, ...], [2, 43, ...]]}

Row values are positional arrays keyed only by the columns header at the top of the file. To answer "what did learner X score", the client must load ~50 normalized files and self-join them on raw auth_user.id integers. Nothing but auth_user.json carries a username or email. A non-technical recipient cannot open any of this.

Note that a straight JSON→CSV swap is necessary but not sufficient: 50 CSVs that are still normalized DB tables joined on integer surrogate keys are barely more usable than 50 JSON files. What the client needs is a small number of denormalized, human-readable CSVs — one row per learner, or one row per learner-per-course — with usernames, emails, course names, grades and completion resolved inline instead of left as FK integers.

It's worse for the ORA chain, which is the actual coursework: submissions_studentitem.student_id and assessment_*.scorer_id hold md5 anonymous ids, not user ids. The client would have to join through student_anonymoususerid to resolve any of it to a person. Their learners' submitted work arrives effectively pseudonymized.

And the data the client would most expect is absent from a MySQL-only export:

• Forum/discussion content is MongoDB (cs_comments_service) — not reachable here at all.
• ORA/SGA learner file uploads live in S3. submissions_submission and assessment_sharedfileupload export the references; the objects aren't copied, so every uploaded assignment points at a file the client doesn't have. staff-graded-xblock is installed here (requirements/edx/base.txt:172), so this covers SGA.
• courseware_studentmodulehistory is in a separate database (student_module_history / edxapp_csmh, per lms/envs/common.py), unreachable via connection — so all per-attempt history is missing.

Meanwhile the Ulmo-compat machinery is dead weight on a client deliverable, and one piece of it actively misleads: the sub_org_id → tenant_id rename emits Koa's sub-org PK under a column name that implies an Ulmo tenant id. Same for auth_user_groups.group_id. Both are plausible-looking wrong integers.

What we already have, and should use instead. Koa already produces exactly that artifact — denormalized CSV, per course, human-readable — battle-tested and exposed on the Instructor Dashboard:

• calculate_grades_csv — lms/djangoapps/instructor_task/tasks.py:181 — one row per learner, columns per assignment, grades resolved
• calculate_students_features_csv (student profile report) — :215 — username, email, name, enrollment mode, cohort
• export_ora2_data — :302 — ORA submissions and scores with learners resolved, not anonymous ids
• plus the problem-responses report

These already solve §2 (CSV, denormalized, readable), §1 (no credential columns), and most of §10's coverage gaps. A far smaller command that resolves the tenant's courses and drives these existing tasks gets the client a usable deliverable with none of §1–§7, and reuses code with real production mileage.

---

The rest applies to the export mechanics regardless of which artifact we settle on, since the same pagination/scoping code would likely be reused.

§3 — BLOCKER: non-terminating loop → disk exhaustion. Operator precedence in the keyset pagination.

_fetch_by_any_id builds a disjunctive WHERE (line 580):

where_sql = u" OR ".join(clauses)      # "`a` IN (%s,...) OR `b` IN (%s,...)"

_paginate appends the keyset predicate by string concatenation, without parenthesising it (lines 756-758):

batch_sql, batch_args = where_sql, list(where_args)
if last_id is not None:
    batch_sql += u" AND {0} > %s".format(bt('id'))

Emitted SQL is WHERE a IN (...) OR b IN (...) AND id > X. AND binds tighter than OR, so MySQL evaluates WHERE a IN (...) OR (b IN (...) AND id > X) — the first clause is unbounded on every batch after the first and re-matches from the beginning. With c1 matching id 1 and c2 matching id 2, at any batch_size:

batch	effective WHERE	rows	last_id
1	c1 OR c2	1, 2	2
2	c1 OR (c2 AND id>2)	1	1
3	c1 OR (c2 AND id>1)	1, 2	2
4	→ identical to batch 2	…	…

batch is never empty, so if not batch: return never fires. The generator yields duplicates forever and _write_table_json streams each straight to disk, so the output file grows until the filesystem fills. On the LMS host that takes the LMS down with it.

Affected — every _fetch_by_any_id call with two non-empty pairs on a table with an id column:

• assessment_assessment (submission_uuid OR scorer_id) — line 660
• assessment_peerworkflow (submission_uuid OR student_id) — line 674
• assessment_peerworkflowitem (scorer_id OR author_id) — line 684
• assessment_staffworkflow (submission_uuid OR scorer_id) — line 690
• assessment_studenttrainingworkflow (submission_uuid OR student_id) — line 707

Trigger: a member who both submitted ORA work and peer/staff-graded. That's every real ORA course — i.e. precisely the case the disjunction was added to support in the last review round.

The test suite cannot catch this. PaginateTests (tests 681-743) only ever passes where_sql='1=1', never a disjunction, and cursor.fetchall.side_effect is a fixed list terminating in [], so the mock can't express non-termination. The suite's own docstring concedes the gap (tests 11-15): "none of the mocked-cursor tests below run the raw-SQL path against a real MySQL instance end-to-end — the backticked identifiers, information_schema introspection, and the keyset ORDER BY … LIMIT pagination SQL built in _paginate are never executed for real."

Fix: batch_sql = u"({0})".format(where_sql) before appending, in both branches; add an iteration cap that raises; add a test asserting the emitted SQL parenthesises the disjunction.

§4 — BLOCKER: unbounded IN (...) binds with no deduplication

Every id list is bound as one flat IN (%s, %s, …), one placeholder per element, and no sink is deduplicated. Worst case is visible_blocks_hashes (line 426-427), which appends one ~100-char hash per subsection-grade row: 10k learners × 50 subsections ≈ 500k entries ≈ a 50 MB single statement, past max_allowed_packet (commonly 4–64 MB) → MySQL server has gone away. visible_blocks_hash is heavily shared across learners, so a set() would shrink it by orders of magnitude; the code uses a list.

Same shape on user_ids (every TABLE_SCOPE query and every --dry-run COUNT(*)), str_user_ids, enrollment_ids (one per enrollment), anon_ids (one per user×course), student_item_ids, submission_uuids, cert_ids, photo_verification_ids.

The docstring flags this as a "known scaling caveat" for the ORA seed lists only (lines 108-112); it applies identically to user_ids in the very first query. Related: because these sinks are unbounded in-memory lists, commit 91720892 ("stream export to avoid OOM") batches the SELECTs but reintroduces the OOM risk via hundreds of MB of accumulated ids.

Fix: chunk every IN bind (~1000/chunk, union results); make every sink a set().

§5 — BLOCKER on the primary: no snapshot isolation, so the bundle is inconsistent by construction

~50 tables, each in its own autocommit transaction, over hours on a real tenant. No transaction.atomic(), no START TRANSACTION WITH CONSISTENT SNAPSHOT, no --single-transaction equivalent — nothing pins a read view.

• A certificate generated after certificates_generatedcertificate is written but before grades_persistentcoursegrade → grade present, cert missing (or the reverse).
• An enrollment created between student_courseenrollment and courseware_studentmodule → studentmodule rows referencing an enrollment absent from the bundle.
• The capture chains are worse: enrollment_ids, cert_ids, anon_ids, photo_verification_ids are snapshots from time T, then used to query dependent tables at T + hours.

The keyset-vs-OFFSET docstring (lines 738-742) reasons carefully about exactly this hazard within one table, then it goes unaddressed across tables — which is where it actually damages a backup.

Fix: wrap the export in one transaction.atomic() (InnoDB's default REPEATABLE READ then gives a stable read view), or run off a restored snapshot.

§6 — BLOCKER on the primary: reads the writer, ignoring this repo's own read-replica convention

from django.db import connection (line 158) is DATABASES['default'] — the primary. Koa already has the plumbing:

• read_replica in DATABASES — lms/envs/common.py:1342
• edx_django_utils.db.read_replica.ReadReplicaRouter — lms/envs/common.py:819
• read_replica_or_default() / use_read_replica_if_available() — common/djangoapps/util/query.py
• already used by an Edly command in this repo — openedx/core/djangoapps/catalog/management/commands/cache_programs.py:75

Streaming millions of courseware_studentmodule rows with KB-sized state blobs off the primary evicts the InnoDB buffer pool and raises latency for a live tenant; stack §5's long transaction on it and you also get history-list growth and purge lag.

Fix: --database, defaulting to read_replica_or_default(); connections[alias].cursor() throughout.

§7 — BLOCKER: no pre-flight table check, and ALL_TABLES — which is exactly that list — is dead code

ALL_TABLES (lines 285-291) is never referenced anywhere in the command (the test file imports it only to assert it has no duplicates). Meanwhile ~50 table names are hardcoded with no existence check: when one is absent, _columns() returns [], columns_sql becomes the empty string, and SELECT FROM \t` WHERE …raisesProgrammingError` mid-export — after N files are written, with the in-flight file left as truncated invalid JSON (§8).

A green --dry-run doesn't protect against this. _dry_run covers TABLE_SCOPE, STRING_CAST_TABLES and the membership table only; it explicitly skips ENROLLMENT_LINKED_TABLES, all 16 ORA_CHAIN_TABLES, the verify_student child, cert invalidation and visible blocks, printing "depends on captured ids from a real run" instead. So the dry run can't detect a missing table in the largest, riskiest part of the export, and gives no size estimate for it either — most of the value of a dry run before a one-shot production job.

Fix: validate all of ALL_TABLES against information_schema up front and fail with the complete missing list before creating any file. Extend --dry-run to cover the chained tables.

§8 — HIGH: mid-job cancel leaves a corrupt bundle indistinguishable from a good one; no atomic finalize, no resume

_write_table_json streams {"columns": …, "rows": [, then rows, then ]}. Ctrl-C / OOM-kill / pod eviction mid-table leaves that file ending mid-array with no closing ]}, while every earlier file looks complete.

Writing MANIFEST.json last is the right instinct, but nothing enforces it as a contract: no "complete": true, no .partial directory, no temp-then-rename, no documented requirement that a consumer reject a manifest-less bundle. Compounding:

• Re-runs mint a new timestamped directory, so partial multi-GB bundles accumulate with no cleanup — disk pressure on the LMS host.
• With an explicit --output-dir, _open_bundle_file uses os.O_TRUNC (line 890): a re-run silently overwrites a previous good bundle, and since the manifest is written last, a re-run that then fails leaves neither a valid old bundle nor a new one. No guard on a non-empty target.
• No --resume; a multi-hour export restarts from zero.

Fix: write into <dir>.partial/, each table <table>.json.tmp → os.replace(), os.replace() the directory on success; "complete": true in the manifest; refuse a non-empty target without --force; --resume skipping tables whose recorded checksum matches.

§9 — HIGH: two silent-success paths, and a docstring that contradicts the code

A collapsed chain reports success. If a seed list captures nothing, _fetch_by_any_id short-circuits to iter(()) (line 579), so all 16 ORA tables write valid 0 rows files, checksums are computed, the manifest is written, and the success banner prints. 0 rows is indistinguishable from "genuinely empty" — total silent loss of the ORA chain presents as a clean run. Add post-export invariants (e.g. submissions_studentitem > 0 while submissions_submission == 0 → warn + non-zero exit) and record seed-list sizes in the manifest.

--batch-size 0 produces an empty backup that reports success. No validation on the argument: LIMIT 0 makes if not batch: return fire on the first iteration of every table → every file 0 rows, manifest written, checksums over empty files, success banner. Negative values throw a raw SQL error. Reject < 1.

MTI docstring is inverted. Lines 733-740 claim keyset pagination "transparently covers Django multi-table-inheritance child tables (e.g. verify_student_softwaresecurephotoverification, assessment_teamstaffworkflow) whose primary key is a *_ptr_id column rather than a plain id." The code tests only if 'id' in columns (line 752); photoverification_ptr_id and staffworkflow_ptr_id are not id, so both MTI children take the OFFSET branch — the branch the same docstring calls non-atomic and unsafe. The two tables claimed safe are exactly the two on the unsafe path, and test_offset_fallback_when_no_id_column (tests 708-714) repeats the claim. Detect the real PK via information_schema.key_column_usage and keyset on it.

§10 — MEDIUM

• Binary data is corrupted silently, and the checksum certifies the corruption. _json_default does value.decode('utf-8', errors='replace') (line 902), permanently substituting U+FFFD for undecodable bytes; the sha256 is then computed over the already-corrupted output, so integrity verification will confirm a corrupted bundle. Base64-encode with a type marker, or raise.
• _json_default is missing uuid.UUID (and timedelta). If the driver returns a UUID object for submissions_submission.uuid, it raises TypeError mid-write, leaving a truncated file. _hex_uuid_to_canonical would also fail on a UUID object, since uuid.UUID(hex=…) requires a string.
• OFFSET branch orders by every column (line 774), including TEXT. That's a filesort, and max_sort_length (default 1024 bytes) truncates the sort key — so rows sharing a long prefix in an earlier text column sort in undefined order and OFFSET pagination can skip or duplicate. Order by the PK.
• Undocumented naive-UTC assumption. Raw SQL bypasses Django's timezone handling, MySQL returns naive datetimes, isoformat() emits no offset. Any consumer parsing these as local time shifts every timestamp in the bundle. Document it.
• Index usability / runtime. WHERE student_id IN (200k values) AND id > X ORDER BY id LIMIT 1000 against courseware_studentmodule will very likely drive off the PK and filter — re-scanning a multi-million-row table once per 1000-row batch, thousands of times, on the primary (§6). Outer-loop over user chunks (~500) so the student_id index is usable, paginate within.
• assessment_assessmentfeedback_assessments is not reference data. It's excluded as an "M2M through-table" alongside rubric/criterion definitions, but its rows are per-learner join records — which assessments a learner's feedback covers. Excluding it orphans every exported assessment_assessmentfeedback row. The dismissal holds for assessmentfeedbackoption (course-authored) and not for the feedback↔assessments join.
• student_manualenrollmentaudit is scoped only by enrollment_id, which is nullable — rows with enrollment_id IS NULL are silently dropped. It carries enrolled_email, which would scope them.
• student_userstanding is missing, so a disabled/banned account restores as fully active — the same class of finding as the accepted is_blocked blocker. Model at common/djangoapps/student/models.py:403. Also absent: student_courseenrollmentallowed, student_usersignupsource, student_languageproficiency, student_socialllink, entitlements_courseentitlement, student_loginfailures, bulk_grades score overrides, and wiki_* (django-wiki is installed → learner-authored content).
• _schema_state swallows every exception (except Exception: return {}), so the manifest's only schema-compatibility signal can vanish silently. MANIFEST.json itself carries no checksum or signature — the per-file digests detect transport corruption, not tampering.
• No confirmation gate or audit trail on a PII-extracting production command: no "about to export N users' PII, confirm" prompt, no --yes, no logger record of operator/row counts/destination.
• Default output path lands under MEDIA_ROOT (_resolve_bundle_dir; MEDIA_ROOT = '/edx/var/edxapp/media/' at lms/envs/common.py:1477). In standard Open edX deployments nginx serves /media/ from that root, which would expose https://<tenant>/media/edm_exports/<slug>_<ts>/learners/auth_user.json. The 0700 mode most likely prevents it since nginx runs as a different uid than edxapp — meaning the protection is incidental rather than designed. I could not confirm the nginx config from this workspace, so please verify against the deployment repo before any run. Commit 91720892 moved the default off bare /tmp for a good reason; MEDIA_ROOT is the wrong destination.
• Nits: self.batch_size assigned in handle() rather than __init__ (pylint W0201); six.text_type on a Python-3-only checkout; --dry-run never prints the resolved output directory.

---

Suggested path forward

Given the deliverable is client-facing CSV, I'd rather redirect this than harden it:

1. Ship the client a CSV bundle built on the existing instructor reports (calculate_grades_csv, calculate_students_features_csv, export_ora2_data, problem responses). A thin management command that resolves the tenant's courses and drives those tasks avoids §1–§7 entirely and reuses code with production mileage. Add the S3 ORA/SGA uploads and, if in scope, forum content — both absent here and both likely expected. If any coverage genuinely isn't reachable through the existing reports, the gap is worth a purpose-built denormalized CSV writer — but it should be scoped to that gap, not a 50-table raw dump.
2. Confirm with legal/compliance what the client is contractually owed before defining scope. That determines whether forum content, uploaded files, and per-attempt history are in or out — none of which this PR covers.
3. If we still want the Ulmo restore bundle, keep it as a separate internal-only command, fix §3–§9, and gate merge on an actual round-trip restore into an Ulmo test tenant. The current test plan's round-trip checkbox is unchecked, so there's no evidence yet that the bundle restores — and independent of that, restoring raw auth_user.id values into a populated Ulmo needs a username-keyed remap. Note the anonymous-id consequence specifically: AnonymousUserId has no unique_together on (user, course_id) (common/djangoapps/student/models.py:130-146) and anonymous_id_for_user salts the digest with SECRET_KEY (:171), so on a target with a different SECRET_KEY the first post-restore access calls get_or_create with a freshly computed digest, creates a second row, and every restored submissions_studentitem.student_id / assessment_*.scorer_id is orphaned — learner ORA history silently reads as absent. Same failure shape as the ORA assessment/rubric remap issue we hit before.

Happy to pair on either direction. If the CSV route is acceptable I can sketch that command.

[ZamanChaudhary]

Thanks for this review — it was right that the JSON bundle was the wrong artifact, and pushing on that reshaped this PR significantly. Here's where every finding in this review landed.

Fully addressed by the redirect

• §1 (secrets in cleartext) — moot by construction now: the CSV export pulls from curated, allowlisted report columns (the same ones the instructor dashboard already exposes to course staff), never SELECT *. meta (the one field that could carry arbitrary app-stashed data) is excluded unless --allow-meta-field is explicitly passed.
• §2 (wrong artifact) — this is the redirect itself. export_learner_data.py (the JSON restore bundle) has been removed from this PR entirely. The new export_tenant_reports_csv command produces per-course and tenant-wide CSVs — grades, profiles, enrollments, problem responses, ORA2 — by calling the same instructor-report generators you pointed at directly, in-process.
• §3 (infinite-loop pagination bug), §4 (unbounded IN queries), §5 (no snapshot isolation), §7 (no pre-flight table check / dead ALL_TABLES) — all specific to the raw-SQL table-dump mechanics of the removed command. That code no longer exists, so these don't apply to what's left.
• §9 (silent success on a collapsed chain) — carried forward as a design principle, not just fixed: any report that comes back with rows but zero survive tenant-membership filtering now sets a warning in that course's status rather than reporting a quiet success.
• §10 (assessment_assessmentfeedback M2M under-scoping, student_manualenrollmentaudit under-scoping, missing tables like student_userstanding) — structurally moot: we're no longer doing a table-by-table raw dump, so "which of 55 tables did we miss" isn't the right question anymore. The report generators' own column sets define scope now.
• Your path-forward argument (AnonymousUserId/SECRET_KEY orphaning on cross-instance restore) — also moot: this artifact is a CSV handoff, not a cross-instance restore bundle, so there's no restore-side id-remapping problem left to solve.

Still open, honestly not addressed yet

• §6 (reads primary DB, ignores the read-replica convention) — not carried over to the new command. It still reads via the ORM's default connection. Worth a follow-up before a real production run at scale.
• §8 (interrupted run leaves an indistinguishable partial bundle) — the new command writes per-course CSVs incrementally, same as the old one; there's no temp-then-atomic-rename step yet, so this risk isn't fully resolved, just no longer paired with the disk-exhaustion failure mode from §3.
• §9 (naive-UTC timestamps), §10 (MANIFEST/schema-state exception swallowing, no confirmation gate/audit trail) — not yet revisited for the new command; flagging rather than assuming carried-over fixes apply.

New gaps introduced by the redirect itself, not in your original review

• The openassessment/edx-ora2 package isn't vendored in this dev checkout, so the ORA2 report's exact column/identity scheme is unconfirmed — mitigated with content-based identity-column detection that skips rather than writes an unfiltered export when it can't verify, with --ora2-identity-column as a manual override.
• No test coverage yet for Command.handle()/--dry-run end-to-end, and no test in this PR has run against a real Django/Celery environment yet (devstack wasn't available while developing this) — that's the next real gate before merge.

Given the scope of this change I'd appreciate a fresh pass whenever you have time, rather than treating the earlier approval as still covering this.

[Waleed-Mujahid]

Fresh pass on 6e14d13. The redirect was the right call — the raw-SQL pagination, information_schema introspection, and 55-table scope-guessing problems are genuinely gone, not papered over, and reusing the platform's own generators is what retired them. Your closure comment was accurate on almost everything.

Keeping this short and only to things that either break something or hand the client wrong data. I've dropped the rest of what I found (bind-list sizes, snapshot-isolation caveats, duplicate per-course files, manifest polish) — none of it stops this working.

Blockers

1. grades_summary.csv has no grade in it. GRADES_SUMMARY_COLUMNS (export_tenant_reports_csv.py:267) omits 'Grade', but CourseGradeReport._grades_header (tasks_helper/grades.py:497) always emits ["Grade"] first, at a fixed offset — only the per-assignment columns after it vary. So the one file a non-technical operator opens to answer "how far did our learners get" has enrollment track and certificate status and zero scores. One-word fix, but it's the whole point of the deliverable.

2. --output-dir chmods a path you didn't create. _make_private_dir (:734-742) does an unconditional os.chmod(path, 0o700), and handle() passes --output-dir through verbatim (:729). --output-dir /edx/var/edxapp/media sets MEDIA_ROOT to 0700 and nginx stops serving every course asset on that box. Only chmod a directory this run created.

3. An interrupted run loses two whole reports and can't be resumed. _DictCsvSink (:375-421) touches the filesystem only in close(), and close() only runs on the normal loop exit at :595. Ctrl-C or an OOM-kill at course 40 of 60 leaves problem_responses.csv and ora2_responses.csv nonexistent (not partial), no MANIFEST.json, and no record of which courses finished — so the only recovery is a full rerun. try/finally around the course loop so the sinks close and the manifest lands with status: incomplete plus the completed-course list would cover this. This is the §8 you flagged as open; I'm raising it to blocking because buffering the two biggest sinks made it worse than the per-table files it replaced.

4. Grade-report error rows are dropped silently. _invoke_report:813 filters out anything ending _err, but CourseGradeReport._upload (grades.py:488-491) uploads two CSVs — grade_report and grade_report_err, the latter holding [user.id, username, error] for every learner CourseGradeFactory failed to grade (grades.py:694). Those learners are absent from the export and the course still reports {'status': 'success', 'rows': N}. The count is already in hand and thrown away: _invoke_report returns the progress dict carrying task_progress.failed (grades.py:475) and _run_grades:971 discards it with result, _ =. Write the error rows to a __grades_errors.csv and put the count in the manifest.

Worth noting no test can catch this one: no fake in the test file emits an _err upload, so the path is invisible by construction — same shape as PaginateTests only ever passing where_sql='1=1' last time.

5. Memory is uncapped for the whole run. _no_max_problem_responses_limit (:304) sets MAX_PROBLEM_RESPONSES_COUNT = None. I checked and it can't crash — grades.py:906 and the decrement both guard if max_count is not None, and the restore is in a finally. But that cap is the only thing bounding this report, and lifting it stacks three live copies: _build_student_data's course-wide list (grades.py:895-968), format_dictlist's copy (:1001), then _DictCsvSink holding every course's rows for the entire tenant until the run ends. ora2 starts worse — collect_ora2_data does list(...) over the whole course then one Assessment query per submission (edx-ora2 data.py:560-578). Make the lift opt-in (--max-problem-responses, defaulting to the platform cap) rather than automatic, so a big tenant doesn't OOM halfway through with no resume path.

Cheap, would do before it leaves the building

• CSV formula injection. QUOTE_ALL doesn't stop Excel evaluating a cell that starts =, +, -, or @, and name/goals/mailing_address/free-text answers are all learner-controlled. The premise here is "a non-technical operator opens this in Excel," so prefix a ' on those. Three lines. The instructor dashboard has the same hole, but that CSV doesn't leave the company.
• --as-user isn't checked for staff. ProblemResponses._build_student_data calls get_course_blocks(user, usage_key) (grades.py:906), which transforms the tree for that user — a non-staff operator gets unreleased/hidden blocks pruned and their responses silently missing, still reported as success. if not user.is_staff: raise CommandError(...) is enough.
• Two known-and-fine-if-stated gaps, worth a line in the manifest rather than code: enrolled_students_features filters is_active=1 (instructor_analytics/basic.py:102-105) while the grade report passes include_inactive=True (grades.py:535-538), so an unenrolled learner lands in grades_summary.csv with a Student ID and no row in learner_profile.csv; and ORA file attachments aren't exported at all (that's upload_ora2_submission_files, misc.py:476, which this doesn't call) — fine if the client's ask is text responses, worth confirming if it isn't.

On the ora2 gap you flagged

Pulled the schema for you from edly-io/edx-ora2@develop-koa (openassessment/data.py:618-633):

Submission ID | Location | Problem Name | Item ID | [Username] | Anonymized Student ID | Date/Time Response Submitted | Response | ...

Your content-based detection lands correctly on it — Submission ID is a UUID, Location a usage key, Item ID an int PK, so they're all disqualified, and the identity columns are the two you'd want. The false positive I went looking for (a scorer column) doesn't exist: scorer identities are embedded inside the Assessment Details text cell (data.py:579), not a column. Username only appears when ENABLE_ORA_USERNAMES_ON_DATA_EXPORT is on (data.py:375-380), which is off by default. So you can name the column directly now and keep the content check as a sanity assert.

On skipping Celery

Agreed, and there's a sharper reason than the ones in the description: these route to HIGH_MEM_QUEUE via GRADES_DOWNLOAD_ROUTING_KEY (lms/envs/common.py:3252), shared with live instructor-dashboard work, and #5's memory profile would OOM-kill a worker holding unrelated tasks. The one thing you gave up is the durable audit trail on the failure path — which is exactly what fixing #3 restores, without bringing the queue back.

Blocking on 1–5. Everything else above is a judgement call I'm happy to leave to you.

[ZamanChaudhary]

Thanks for this — every one of the 5 blockers, the 2 non-blocking items, and both documentation gaps are fixed.

• Blocker 1 (missing Grade column) — grades_summary.csv now includes it.
• Blocker 2 (chmod on a shared dir) — _make_private_dir now only locks down directories this run actually created.
• Blocker 3 (interrupted run loses data + manifest) — reworked so an interruption always leaves a manifest behind, and both buffered report types (problem_responses, ora2) survive a partial run. Went further than the original ask here: the same "don't report a false success" principle now also covers a failing sink close and a failing manifest write individually, and per-course report failures are now rolled up to a top-level courses_with_errors / complete_with_errors status instead of the run silently reporting clean success when a course actually failed.
• Blocker 4 (dropped grade-report error rows) — those learners now land in <course>__grades_errors.csv, tenant-filtered, with the failure count recorded.
• Blocker 5 (automatic memory-cap lift) — now opt-in via --max-problem-responses, off by default.
• CSV formula injection — escaped on all three write paths.
• --as-user staff check — added; a non-staff operator is now rejected rather than silently getting a pruned problem_responses export.
• Docs — the active/inactive enrollment mismatch between learner_profile.csv and grades_summary.csv, and the ORA2 file-attachment exclusion, are both now documented in the module docstring and surfaced in the runtime manifest, not just source comments.

Also closed the three gaps disclosed after the last redirect: the command's own queries now go through read_replica_or_default() (the five upstream report generators deliberately stay on the primary — CourseGradeReport has a real read-then-write path that's unsafe against replica lag, documented rather than silently routed), and Command.handle()/--dry-run now have test coverage.

One honest caveat: none of this has run under a real Django test environment yet — no working devstack in the environment this was built in — so that's still the next real gate before merge, not something I'm claiming is done.

[Waleed-Mujahid]

Checked 576a2d1e against real upstream source rather than taking the closure comment at its word. All five blockers are genuinely fixed, and two of them I could only confirm by reading the generators they depend on:

• 1 (missing Grade) — 'Grade' is in GRADES_SUMMARY_COLUMNS at index 3. Worth noting why the header.index(col) validation at :1569 is safe: all ten of those columns come out of CourseGradeReport._success_headers (tasks_helper/grades.py:441-450) unconditionally — only Cohort Name, the experiment-partition columns, and Team Name are conditional, and none of those are in your allowlist. So there's no course shape that makes this raise.
• 4 (dropped error rows) — _error_headers() returns exactly ["Student ID", "Username", "Error"] (grades.py:452), so _filter_rows_by_column(error_header, ..., 'Student ID', ...) matches the real header rather than landing in the grades_errors_export_error branch on every run with a failure. That was the way this fix could have been quietly useless; it isn't.
• 2 — chmod is now inside the if not os.path.exists. 3 — try/finally closes the sinks and the manifest lands as incomplete. 5 — opt-in via --max-problem-responses, _null_context() by default so FEATURES is untouched.

On the tests: the specific blind spot from last round is closed. Last time no fake emitted an _err upload, so blocker 4's code path was invisible by construction — test_grade_report_err_rows_are_captured_and_failure_count_recorded now emits both uploads from one call, and test_grades_error_row_failure_does_not_flip_a_successful_course_to_error covers the isolation. test_open_sinks_partial_failure_still_closes_and_records_the_already_opened_sinks asserting sink._file.closed on the two that opened is the right shape too.

Dropping my block. Three conditions before merge, in order:

1. Run the tests. This is the real gate and you already named it. 3,680 lines that have never executed. I couldn't run them either (no Koa devstack here), so I cleared what's clearable statically instead — the results are worth having before you spend devstack time:

• read_replica is absent from lms/envs/test.py:185's DATABASES, which only defines default and student_module_history. So read_replica_or_default() returns 'default' under test settings. That matters a lot: had test settings kept the alias, every .using(read_replica_or_default()) call would open a second connection that can't see a TestCase's uncommitted fixtures, and roughly every DB-touching test in this file would have failed for a reason that has nothing to do with your logic. ReadReplicaRoutingTests is fine as written because it asserts the call, not the alias.
• EdlySubOrganizationFactory / EdlyMultiSiteAccessFactory exist at openedx/features/edly/tests/factories.py:29,65. from student.tests.factories import ... (not the common.djangoapps. prefix) matches all eight sibling files in openedx/features/edly/tests/. tests/__init__.py is present at the PR head. Both files compile clean.

No known import blocker left — but that's static analysis, not a green run.

2. --dry-run is broken as documented. The --as-user/is_staff gate (:737-746) sits above the if dry_run: branch (:755), and DEFAULT_REPORTS includes problem_responses — so the module docstring's own first Usage line, manage.py export_tenant_reports_csv <slug> --dry-run, exits with a CommandError. You hit this and documented the workaround in the test instead of fixing it (test_dry_run_writes_nothing's comment: "without this, --dry-run with no --as-user would raise CommandError before ever reaching the dry-run branch"). A test comment explaining why the happy path can't be tested as documented is the signal to move the branch. --dry-run writes nothing and calls no generator, so it has no business needing an operator identity — move the if dry_run: block above the gate.

3. Don't default the output under MEDIA_ROOT. _resolve_output_dir falls back to <MEDIA_ROOT>/edm_exports/... = /edx/var/edxapp/media/, which nginx serves at /media/. The 0700 dirs and 0600 files probably hold if nginx runs as a different user than edxapp, but "probably, depending on deployment" isn't the property you want on a directory holding a whole tenant's learner PII, and the timestamp in the path is guessable within a range. Default somewhere outside the web root, or make --output-dir required.

One thing that isn't a blocker but is worth hearing: the code got better this round and the prose got worse. The module docstring is 361 lines against ~1,090 lines of actual code, and the finally block carries 40+ lines of comment explaining its own control flow, including comments about what the code used to do and which review round changed it. Rationale that deep belongs in the commit message — that's where someone doing git blame in a year will look for it, and it doesn't have to be re-read by everyone who opens the file. The code reads well enough now that most of it isn't earning its space.

Last thing, procedural: @muhammadali286's empty approval from 2026-07-29 is still on this PR. With my block dropped, that's now a second approval on a branch whose tests have never run — worth making sure nobody merges on it before condition 1 is done.

[ZamanChaudhary]

Thanks for the approval, and for catching these two on the deeper pass — both are fixed now:

• --dry-run ordering — the --as-user/staff check for problem_responses was running before the dry-run early return, so the exact usage example in this command's own docstring (--dry-run with default flags, no --as-user) raised CommandError. Reordered so dry-run always short-circuits first, since it never touches an operator identity or calls a generator. test_dry_run_writes_nothing no longer works around this with --reports grades — it now exercises the actual documented default invocation.
• Output directory under MEDIA_ROOT — you're right that EDM_EXPORT_DIR isn't defined anywhere in this codebase, so that fallback never actually helped, and the MEDIA_ROOT default was a live exposure, not a latent one. Made --output-dir required instead of trying to guess a safer default — there wasn't one to fall back to.

On your first condition — actually running the ~3,680 lines of tests on real devstack — that's still open on my end and isn't something I can simulate away; I've confirmed as much of it as this environment allows (a real Django 2.2.24 venv probe for the required-argument behavior, plus the standalone harness), but a genuine devstack run is still the real gate before merge.

Left the prose/comment-bloat note as-is for now since you flagged it as non-blocking — happy to do a pass on that separately if useful.