Skip to content

AGENTS.md

Latest commit

 

History

History
266 lines (223 loc) · 16.8 KB

AGENTS.md

File metadata and controls

266 lines (223 loc) · 16.8 KB
Raw

AGENTS.md

Guidance for AI coding agents working in this repository.

What this repo is

Open edX Core (PyPI: openedx-core, formerly openedx-learning / "Learning Core") is a library of Django apps that openedx-platform and community plugins install. It is not a runnable service: projects/ and manage.py exist only for local development and tests. Code here must never import from openedx-platform.

Packages live under src/ and are importable without the prefix (import openedx_content). Tests live in tests/<package>/..., mirroring src/, and are not published.

Commands

Python 3.12, Django 5.2, dependencies managed with uv (uv.lock). After make requirements (= uv sync --group dev), tools are available via uv run <cmd> or .venv/bin/<cmd>.

Task Command
Install dev deps make requirements
All tests (SQLite, with coverage) make test or pytest tests
One file / one test, no coverage pytest tests/openedx_content/applets/units/test_api.py --no-cov -k test_name
Tests against MySQL (what CI runs) tox -e py312-django52 (needs MySQL 8 on 127.0.0.1:3306; docker recipe in mysql_test_settings.py)
Lint + type-check everything make quality (= tox -e quality)
Fix import ordering isort src tests test_utils
Import-boundary check only lint-imports
Type-check only mypy
PII annotation check make pii_check
Build docs tox -e docs
Tests + quality + pii make validate
Check for missing migrations python manage.py makemigrations --check --dry-run
Generate a migration python manage.py makemigrations <app_label>
Refresh uv.lock make upgrade

Notes:

  • pytest reads DJANGO_SETTINGS_MODULE = test_settings (in-memory SQLite) from tox.ini. An exported DJANGO_SETTINGS_MODULE env var (e.g. inside a tutor container) overrides it; pass --ds=test_settings explicitly in that case.
  • CI runs the suite against MySQL via mysql_test_settings, so tests that pass locally on SQLite can still fail in CI. Watch query counts (assertNumQueries is used heavily) and anything that depends on ordering, collation or case sensitivity.
  • make quality runs pylint, mypy (django-stubs plugin, settings projects.dev), pycodestyle, pydocstyle (on src only), isort --check-only, and lint-imports. Line length is 120.
  • pylintrc and .editorconfig are generated by edx-lint. Put repo-specific changes in pylintrc_tweaks and run edx_lint write pylintrc; do not edit the generated files. Likewise constraint-dependencies in pyproject.toml is overwritten by make upgrade; repo-specific constraints go in [tool.edx_lint].uv_constraints.
  • pii_check is not in the CI matrix, but every model needs either a .. no_pii: (or .. pii: block) annotation in its docstring. External models that are imported for use in this repo should have an entry in .annotation_safe_list.yml.
  • Ruff is configured in pyproject.toml as the recommended editor formatter, but it is not in any dependency group and is not part of the quality gate.

Commits and releases

Commit messages must follow Conventional Commits; a commitlint workflow checks every PR. Every push to main runs python-semantic-release, which derives the next version from commit types (feat: minor, fix:/perf: patch) and publishes to PyPI. The type prefix on a squash-merge commit therefore decides whether and how a release happens.

Databases

Code must work on SQLite, MySQL, and PostgreSQL. MySQL is the primary deployment database and is always run at the READ COMMITTED transaction isolation level. Tests run on SQLite locally and on MySQL in CI; nothing exercises PostgreSQL automatically, so write vendor-neutral ORM code by default and gate any exception explicitly.

Consequences of READ COMMITTED, and how to handle concurrency:

  • A transaction does not see a consistent snapshot. Two reads of the same rows inside one transaction.atomic() block can return different results if another transaction committed in between.
  • "Check, then insert" is not protected against a concurrent insert. Settle races with database constraints, not locks: the (entity, version_num) unique constraint on PublishableEntityVersion exists precisely so that one of two concurrent editors gets an IntegrityError. Catch that rather than pre-checking for existence. Unique constraints, check constraints, and conditional updates (filter(...).update(...) and checking the returned row count) all catch races without holding a lock.
  • Avoid select_for_update(). The platform is meant to run at scale, and a row lock held for the rest of a transaction serializes every concurrent writer on those rows while Python code runs, with lock waits compounding under load. Treat it as a last resort that needs a comment explaining why a constraint can't do the job, never as the default answer to a race.
  • Don't design around behavior that REPEATABLE READ would give for free (stable re-reads, gap locks on range scans); deployments won't have it. Similarly, don't make cautionary comments around REPEATABLE READ behavior, since it is irrelevant to this repo.

Known vendor differences and the existing patterns for them:

  • Case sensitivity and collation. MySQL compares text case-insensitively by default; SQLite and PostgreSQL do not. Use the helpers in openedx_django_lib.fields (case_sensitive_char_field(), case_insensitive_char_field(), code_field(), ref_field()), which build a MultiCollationCharField with a per-vendor db_collations dict. The MySQL collations are utf8mb4_unicode_ci / utf8mb4_bin for MariaDB compatibility; don't use utf8mb4_0900_*. Those dicts currently have no postgresql entry, so a "case-insensitive" field falls back to PostgreSQL's default (case-sensitive) collation there.
  • Upserts. bulk_create(update_conflicts=True) requires unique_fields on PostgreSQL and SQLite but rejects it on MySQL; branch on connection.features.supports_update_conflicts_with_target as publish_from_drafts does. Prefer connection.features flags over connection.vendor checks whenever Django exposes one.
  • Index key length. InnoDB caps index keys at 3072 bytes, i.e. 768 utf8mb4 characters. A long indexed CharField needs a prefix index on MySQL and a normal index elsewhere; see openedx_tagging migration 0020 for the SeparateDatabaseAndState + RunPython pattern that branches on schema_editor.connection.vendor while keeping makemigrations quiet.
  • Unique constraints and NULL. NULL is the only value all three databases exempt from a unique constraint; empty strings and sentinels are not. If NULL has to be treated as unique for the purposes of a composite unique constraint, then create a derived CharField that serializes the values of the covered fields (with NULL as "") using separators, and make that derived CharField unique.
  • Types. PostgreSQL has no unsigned integers; stick to Django's standard field types.
  • Ordering. Default row order and collation-driven sort order differ between vendors. Add an explicit order_by() wherever order matters, including in tests.

Architecture

Layered top-level packages

Each top-level package in src/ is a separate Django app (or utility package). Import-linter (.importlinter, run by lint-imports) enforces that a package only imports from packages below it:

openedx_learning    learner-domain features: Competency-Based Education (cbe), Pathways
openedx_content     authored, versioned content: publishing, components, containers, media, ...
openedx_catalog     the enrollable things: CatalogCourse, CourseRun, CatalogPathway, PathwayEnrollment
openedx_tagging     taxonomies, tags, object tags (generic; must never learn that CBE exists)
openedx_django_lib  Django helpers (typed ID fields, collation-aware fields, ReadOnlyModelAdmin); internal, not public API
openedx_core        empty shell exposing __version__; intentionally has no API

Key consequence: openedx_catalog knows nothing about content. openedx_content holds foreign keys to catalog models, never the reverse (catalog ADR 0001). Don't "fix the build" by loosening .importlinter; the comments in that file explain each boundary.

Per-app public API modules

Consumers (openedx-platform, plugins) import only from these modules of each app:

  • api.py: the functions that create, modify and query data. All mutation goes through here. Functions not marked "UNSTABLE" in their docstring are stable; breaking them requires the community DEPR process. This layer is plain Python, called from views, Celery tasks, management commands, and other apps' APIs alike, so it must never assume an HTTP context: no get_object_or_404, no Http404, no rest_framework imports, no request parameters. Report failures by raising Model.DoesNotExist, Django's ValidationError, or an app-specific exception, and leave it to the view to translate those into responses (the CBE views do this by re-raising DRF NotFound / ValidationError).
  • models_api.py: the models callers may use as foreign-key targets, in type annotations, or to extend. Treat them as read-only from outside the app; never instantiate or save them directly.
  • data.py (where present): value objects / TypedDicts passed across the API boundary.
  • rest_api/, urls.py, signals.py, settings_api.py where present. settings_api.py must not import models because it runs during Django settings evaluation.

openedx_catalog additionally has an isolated_apps import-linter contract (test_utils/isolated_apps_contract.py): code outside it may import only its api, models_api, data and tests modules. This is a pattern we want to work towards for all our apps.

Applets inside openedx_content and openedx_learning

openedx_content and openedx_learning are each a single Django app (one app label, one migrations/ directory) whose internals are split into "applets" under applets/<name>/. An applet looks like a miniature app: its own api.py and models.py, and optionally admin.py, signals.py, rules.py, rest_api/. The app-level api.py, models.py, models_api.py, admin.py, signals.py and rules.py are glue modules that wildcard-import from every applet, relying on each applet module's __all__ to define what is public.

Rules for working in applets:

  • Applets call each other only through the other applet's api.py, never by querying its models directly. Within an app, an applet may import a sibling's api function even if it isn't in __all__ (provisional API); underscore-prefixed names are fully private.
  • Applet ordering inside openedx_content is enforced by the content_applet_layering contract: api > backup_restore > components | containers > media > collections > publishing. publishing is the foundation and depends on nothing else.
  • Adding an applet means: add it to the right layer in .importlinter, add its modules to the app-level wildcard import lists, register any publishable model pairs in ContentConfig.register_publishable_models(), and (for openedx_learning) re-export its rules.py from src/openedx_learning/rules.py, because rules.apps.AutodiscoverRulesConfig imports only <app>.rules.
  • openedx_content/backcompat/ holds shells of the pre-merge apps (oel_publishing, oel_units, ...): their old migrations plus skeleton models, kept so openedx-platform's existing migrations still resolve. Consumers install them via openedx_content_backcompat_apps_to_install(). Never add new code there; ADR 0010 explains the migration choreography.

The publishing model (openedx_content)

Everything content-related builds on the publishing applet:

  • LearningPackage is the unit of isolation (a library or, by convention, one course run; ADR 0012). Codes are unique within a package, Media rows are deduplicated by hash within a package, and nothing is shared across packages.
  • PublishableEntity + PublishableEntityVersion is the versioning primitive. A version belongs to exactly one entity (no copy-on-write, ADR 0011). Draft and Published are one-row-per- entity pointers to the current draft/published version; a NULL version means deleted.
  • Concrete content types (Component; Container and its subtypes Unit, Subsection, Section) are 1:1 with PublishableEntity / PublishableEntityVersion via PublishableEntityMixin / PublishableEntityVersionMixin, and are registered in ContentConfig.ready().
  • Containers (ADR 0007): a ContainerVersion has an ordered EntityList of EntityListRows; each child is pinned to a specific version or None for "latest". Changing a container's children, order or metadata makes a new container version; editing a child does not cascade up. Publishing a container publishes its draft children. Restrictions on child types (units hold components, etc.) live in the subtype applets, not in containers.
  • Every draft change is recorded in a DraftChangeLog with one DraftChangeLogRecord per entity plus DraftSideEffect rows linking each changed child to its immediate parent container (one level only, not transitive); publishing produces a PublishLog the same way. Use bulk_draft_changes_for(learning_package_id) to batch many edits into one change log and one transaction; it wraps a private context-var-based manager in publishing/contextmanagers.py. These logs drive the openedx_events signals exposed as openedx_content.api.signals.

Extensibility conventions

Other apps and plugins extend core models with their own related models (OneToOneField(..., primary_key=True) is the common pattern), not by subclassing concrete models (ADR 0002). That is why models_api.py exposes mixins and models: to be joined to and extended, while all creation and mutation stays in api.py. Django admin classes should subclass openedx_django_lib.admin_utils.ReadOnlyModelAdmin for the same reason.

Model field conventions (see README): id is the BigAutoField primary key and the only foreign-key target; uuid is the stable external identifier; key/num (db column _key) are client-meaningful and may change. Models declare a typed primary key (Model.ID NewType via TypedBigAutoField), and api.py functions take LearningPackage.ID, PublishableEntity.ID, etc.

Settings that get baked into migrations must match openedx-platform, where these migrations actually run; SIMPLE_HISTORY_DATE_INDEX = False in both settings files is one such case.

openedx_learning and openedx_tagging specifics

  • CompetencyTaxonomy is a multi-table-inheritance subclass of tagging's Taxonomy sharing its primary key. Ask "is this a competency taxonomy?" via openedx_learning.api (is_competency_taxonomy() / select_competency_taxonomies()), never via taxonomy.competencytaxonomy, so tagging stays unaware of CBE.
  • Permissions use django-rules, always checked through user.has_perm(...) rather than by calling a predicate directly, because openedx-platform swaps in org-aware rules via rules.set_perm and a direct call skips them. Tagging's api.py enforces no permissions (its views do); some CBE API functions take a user and raise Django's PermissionDenied themselves. A consuming project needs rules.apps.AutodiscoverRulesConfig in INSTALLED_APPS and rules.permissions.ObjectPermissionBackend in AUTHENTICATION_BACKENDS; without them every non-superuser gets 403 with nothing in the logs, and testing as a superuser hides the problem.
  • REST APIs: tagging mounts openedx_tagging.urls (app_name oel_tagging); learning mounts openedx_learning.urls, which serves CBE at <prefix>cbe/v1/. The tagging app label is oel_tagging (legacy prefix, so its tables are oel_tagging_*); openedx_content and openedx_learning use unprefixed labels.

Tests

  • Two styles coexist: older openedx_content tests use Django TestCase with setUpTestData and build on base classes such as ComponentTestCase; newer tests (CBE, catalog) are pytest functions with pytestmark = pytest.mark.django_db and fixtures from the nearest conftest.py. Match the style of the module you are editing.
  • tests/utils.py provides capture_events() for asserting on emitted openedx_events signals and abort_transaction() for rolling back a block. tests/openedx_content/conftest.py makes Celery tasks run eagerly.
  • tests/test_django_app holds test-only models (TestEntity, TestContainer, ...) for exercising the generic publishing and containers APIs without the concrete content types; such classes carry __test__ = False so pytest doesn't collect them.

Docs and ADRs

Design decisions are recorded as numbered ADRs under docs/<package>/decisions/, and significant model or API changes should come with one. Each applet also has a readme.rst stating its motivation and architecture guidelines.