Contributing

This page mirrors CONTRIBUTING.md from the repository and is regenerated on every merge to main. To correct it, edit that file; changes made here are overwritten.

This is a short document about how this codebase is written, not a generic PR-process guide. It exists because docs/archive/ENGINEERING_PRODUCT_UX_REVIEW_2026-08-20.md §4 named two habits as this project's single strongest asset — stronger than any individual feature — and flagged them as fragile precisely because they're emergent conventions, not enforced rules. Writing them down is the cheapest possible insurance against losing them as more people touch this code.

1. Comments explain why, not what

A comment that restates the code adds nothing. A comment that explains the failure mode the code is defending against is worth its weight — it's the difference between a future contributor "fixing" a workaround back into the bug it was written to avoid, and that contributor understanding why the workaround exists before touching it.

Concrete examples already in this codebase, worth reading before writing your own:

• ui/canvas_highlight.py: "QgsHighlight has no overload that accepts a raw QgsRectangle — only QgsGeometry or QgsFeature. Passing the rectangle directly raised 'arguments did not match any overloaded call' on every single highlight attempt (confirmed live)."
• agent/providers/gemini.py: "a live user hit a 404 'no longer available to new users' on gemini-2.5-pro even though Google's own docs still listed it as stable at the time."
• ui/dock_widget.py: "QTabWidget/QStackedWidget sizes the WHOLE dock to its tallest tab's natural size hint, not just the currently visible tab... forcing the entire QGIS window taller than the screen."

Each of these cites a real, specific, previously-hit failure — not a hypothetical, not "just in case." When you fix a real bug, write the comment the way these are written: what broke, how you know it broke (a stack trace, a live user report, a test that failed), and why the fix works. When you're tempted to write a comment that just describes what the next line does, skip it — the code already says that.

2. State what's shipped, roadmap, and unverified — honestly, in the code itself

This project's docs are full of sentences like "not yet built," "roadmap only," "unverified against a live response in this environment." That's not hedging — it's the reason docs/archive/STATUS_REVIEW_2026-08-20.md can be trusted as an accurate picture of the product instead of aspirational marketing that quietly drifted away from the real codebase.

This discipline belongs in code, not just docs. agent/providers/cartogen.py's module docstring is the clearest recent example — it states plainly that it's a stub, lists exactly what it is not wired into, and says explicitly not to extend it until specific preconditions are met. That's a higher bar than most projects hold internal comments to, and it's the right bar here: a comment or docstring that overstates what a piece of code does is actively worse than no comment at all, because it's the thing a future contributor (or a future AI-assisted session) will trust without re-checking.

In practice:

• If something is a stub, a prototype, or unverified against a live/real environment, say so in the code that implements it, not only in a separate doc someone might not read.
• If you're not sure something works — because it can't be tested in this sandbox, or you haven't run it against a real API/QGIS session — say that too, specifically (see docs/RELEASE_SMOKE_TEST.md for the live-verification gap this can't close alone).
• Don't let a docs file's status label go stale. When you ship something a doc previously called "not yet built," update that doc in the same change — this project's own review rounds have repeatedly found stale status labels as real, fixable gaps precisely because this step gets skipped under time pressure.

3. When you find a gap, flag it — don't silently fix it if it's a judgment call

Several review rounds in this project's history found real issues and did not fix them unilaterally — redundant tool registrations, a missing raster styling tool, an inconsistent auth mechanism in a provider client — because the fix involved a product or design tradeoff, not just a mechanical correction. Mechanical, low-risk fixes (a wrong error message, a missing retry) get fixed directly. Anything that changes behavior a user or integration might depend on, or that this environment can't verify live, gets flagged with a clear recommendation instead of applied silently. If you're not sure which category something falls into, treat it as the second one.

4. Testing conventions

• Every module that touches qgis.core/PyQGIS degrades gracefully outside a real QGIS process via its own QGIS_AVAILABLE guard, so the test suite runs without a QGIS installation (python -m unittest discover -s tests -t . -p "test_*.py" — the -t . matters, see CLAUDE.md's "Running things" section). New tool modules should follow the same pattern — see any file in src/cartogen_ai/core/agent/tools/ for the shape.
• Pure-Python logic (chat formatting, color derivation, tool routing) is deliberately kept free of Qt/QGIS imports specifically so it's directly unit-testable — see src/cartogen_ai/core/ui/chat_formatting.py's own docstring for why it's structured that way relative to src/cartogen_ai/core/ui/dock_widget.py.
• A structural bug this project actually shipped and caught (a duplicate dict key silently discarding ToolRouter aliases, with no error at parse or runtime) is now guarded against by an ast-based static test (tests/test_tool_router.py's TestToolAliasesNoDuplicateKeys). If you add a similarly "the interpreter won't warn you" class of footgun, consider whether a structural test like this one is warranted, not just a behavioral one.

5. Single tree, one public codebase

This repo is a single source tree — there's no second copy to keep in sync (an earlier version of this project maintained a dual-tree setup; see CHANGELOG.md's [1.4.0] entry). It's a single, open-source GPL v2 codebase. Do not add tier-check or license-gating logic here — that's out of scope for this repo by design.

6. Release tag naming: cartogen-ai-v<version>, not commercial-plugin-v<version>

Every release tag through commercial-plugin-v1.16.0-rc6 used the commercial-plugin- prefix. That name described the intended distribution channel (services-based monetization — support, hosting, custom integration — rather than code licensing; see CHANGELOG_ARCHIVE.md's [1.0.0-beta] entry, 2026-08-11), not the license: this codebase has been GPL v2 since that first tag. Read on a repo whose own LICENSE says GPL v2, a tag literally named commercial-plugin- reads as a proprietary-licensing signal it was never meant to send.

Decided 2026-09-27 by Alaa: new tags use cartogen-ai-v<version> going forward (e.g. cartogen-ai-v1.17.0, matching metadata.txt's version= value). Existing commercial-plugin-v* tags are untouched — they're referenced by already-published GitHub Releases and by this repo's own historical record (CHANGELOG.md's past entries, docs/IMPLEMENTATION_TRACKER.md's dated journal entries); per §7 below, that history isn't rewritten. Don't retag or rename old releases to match; just use the new prefix for the next one.

7. Frozen historical docs

docs/archive/CARTOGEN_AI_FEATURE_LIST.md, docs/archive/CARTOGEN_AI_PRD.md, docs/archive/IMPLEMENTATION_TASK_LIST.md, CHANGELOG.md's past entries, and dated review/ audit/spec docs (filenames ending in a date, e.g. docs/archive/STATUS_REVIEW_2026-08-20.md) are deliberately left untouched after the fact — they're a historical record, not living documentation. If something in one of them is now wrong or superseded, add a new dated doc or a docs/IMPLEMENTATION_TRACKER.md entry that supersedes it; don't edit the old one to match current reality. This includes not "fixing" old identifier names in them (e.g. QgisAiAgent) even after a rebrand — they're accurate to what was true when written.

8. Releasing: every version bump updates the user-facing text

When you change version= in metadata.txt, update ALL of these in the same change -- tests/test_release_docs_in_sync.py fails until they agree:

1. metadata.txt: prepend a changelog= block for the new version (never edit older blocks). Never write the % symbol anywhere in this file; write "percent" -- plugins.qgis.org parses it with Python's ConfigParser and rejects the upload (tests/test_metadata_parses.py and plugin_upload.py both check; 2026-10-10 the directory refused rc24 because of a "25% keyword coverage" in an old block, which was reworded to "25 percent" -- the one exception to "never edit older blocks").
2. CHANGELOG.md: a new row in the "Recent releases" table and a new ## [version] section with its <a id="v..."> anchor.
3. src/cartogen_ai/core/release_notes.py: WHATS_NEW_VERSION, NEW_MARK_VERSIONS, WHATS_NEW_ITEMS (shown in the plugin's Help dialog), any new tool in HUMANITARIAN_WORKFLOWS with its TOOL_NEW_IN entry.
4. README.md: the version line, the status line, a new "What's new in " section (keep the previous one below it), and the humanitarian table -- regenerate it from release_notes.render_readme_table().
5. Counts: the offline test count in the README status block (take it from the last green offline run, never from memory), tool count (README, docs/TOOLS_REFERENCE.md via python docs/generate_tools_reference.py; tests/test_tools_reference_in_sync.py fails if it is stale) and task/section counts (docs/USER_GUIDE.md); the Help dialog reads them live.
6. Say plainly what was and was not verified (hand-tested vs CI only), then build the zip with python plugin_upload.py.

9. QGIS / PyQGIS development rules

These follow the PyQGIS Developer Cookbook, QGIS 4.2 source (docs/pyqgis_developer_cookbook/ on the release_4.2 branch of qgis/QGIS-Documentation, commit 12332b8, read 2026-10-11; the rendered site docs.qgis.org was not reachable from the sandbox, the git source was). Chapters read: tasks, settings, communicating, authentication (outline and warnings), loading layers, vector (editing, providers, repaint), composer (layouts, export), processing, plugins, releasing; geometry, crs and raster were searched for rules, not read in full. The cookbook has no HTTP chapter (network_analysis is about road graphs), so the network rule is our own. This plugin targets QGIS 4.2.x / Qt6 only; where the cookbook shows a flat QGIS enum such as QgsTask.CanCancel, prefer the scoped form and use resolve_qgis_enum when a member may have moved.

1. Threads (Tasks chapter). A background task must NEVER touch a QObject that lives on the main thread: no QgsVectorLayer, no QgsProject, no widgets. Data a task needs is copied before it starts. The QgsProcessingContext and QgsProcessingFeedback given to a task must outlive it (QGIS crashes otherwise). In QgsTask.run() never raise: store the exception and return False, and re-raise or report it in finished(), which always runs on the main thread. Poll isCanceled(), return True/False, and give the user a Stop (tools/_background_processing.py, cancel_signal.py). The global task manager is shared, so name tasks clearly.
2. Network. Use core/net.py (QgsNetworkAccessManager/QgsBlockingNetworkRequest) so QGIS proxy, SSL and authentication settings apply. New code does not call requests/urllib directly; the two kept deliberately (SSRF-guarded opener, Ollama) are in SECURITY.md §4a. Always set a timeout and a size cap. (Our own rule: the cookbook chapter on this was not read.)
3. Credentials and settings (Settings and Authentication chapters). There are three kinds of setting, use the right one: user-global QgsSettings (keys namespaced under cartogen_ai/ via settings_keys.py), per-project QgsProject.writeEntry/readEntry* (read back with the matching typed reader), and per-layer setCustomProperty. Secrets never go in any of them: the QGIS Auth Manager or session memory only (infrastructure/auth.py). Only a subset of the auth manager is exposed to Python by design, so do not work around that, and leave authcfg expansion to QgsAuthManager; never expand it yourself into a URI early.
4. Layers and data (Loading layers chapter). Check isValid() after every vector or raster load, and build URIs with QgsDataSourceUri, which escapes special characters. After changing a layer through its data provider (changeAttributeValues, addAttributes), call layer.updateFields() and layer.triggerRepaint(): provider edits do not repaint by themselves (Vector chapter note), and triggerRepaint() also clears the cached canvas image. Editing-buffer changes go through startEditing and commitChanges or with edit(layer) (which rolls back on error and raises QgsEditError when the commit fails), and wrap them in beginEditCommand/endEditCommand if the user should be able to undo; tools/_edit_session.py checks every result. Layout exports compare the QgsLayoutExporter result with ExportResult.Success. Our own practice: validate a CRS before reprojecting, use the project's transformContext() and ellipsoid, check the result of commitChanges, roll back on failure, and prefer processing.run("native:...") over hand-written geometry loops.
5. User messages (Communicating chapter). Message boxes are poor UX: use iface.messageBar().pushMessage(..., level=Qgis.MessageLevel.X) for notices and warnings, with a duration for transient ones, from the GUI thread only. Inside our own dialogs use core/ui/dialog_messages.py push_dialog_message (a QgsMessageBar in the dialog, as the chapter shows). QMessageBox stays only for questions that need an answer and for the credential-storage warning in settings_dialog.py, which the user must acknowledge. Progress can go in the message bar or QgsTask progress. Internal logging uses core/logger.py log_event (metadata only). No print() (ruff T20).
6. Qt6 and enums. Scoped enums (Qt.AlignmentFlag.AlignLeft, Qgis.MessageLevel.Critical), imports through qgis.PyQt, never PyQt5/PyQt6 directly, no exec_.
7. Plugin lifecycle (Plugins chapter). The plugin class provides __init__(iface), initGui() and unload(); classFactory(iface) is in the root __init__.py. unload() must undo everything initGui() did: menu items, toolbar icons, dock, processing provider, signal connections (the cookbook example disconnects the canvas signal), timers and tasks. Load, unload and reload must work without restarting QGIS (CI smoke test).
8. Processing (Processing chapter). metadata.txt has hasProcessingProvider=yes; initProcessing() calls processingRegistry().addProvider(...) from initGui, and unload() calls removeProvider(...). Algorithms are QgsProcessingAlgorithm subclasses in processing/, use feedback for progress and cancel, and make no GUI calls. The Releasing chapter also recommends offering analyses as Processing algorithms so users can batch them.
9. Packaging and metadata (Plugins and Releasing chapters). Required metadata: name, qgisMinimumVersion, description (one line, no HTML), about, version (unique per upload), author, email, repository; also tracker, homepage, tags and a real icon. category may only be Vector, Raster, Database, Mesh or Web, or be left out (we leave it out: the plugin sits in the default Plugins menu). No % in the file (§8). All metadata UTF-8. A LICENSE file with no extension at the package root. The package is one top folder containing metadata.txt, __init__.py and LICENSE; the folder name uses only ASCII letters, digits, _ and - and does not start with a digit (we use lower-case snake_case, which the directory's own validator also requires). No generated files (ui_*.py, resources_rc.py), .gitignore, .pyc, __pycache__, hidden files or .. paths. Zip at most 25 MB. tests/test_directory_validator.py covers most of this. Translations, if added, are named <plugin>_<2-letter language>.ts and the .ts and .qm files are regenerated after UI changes.
10. Dynamic code. No eval/exec of model- or user-supplied text in the plugin process. The one script tool runs in the isolated process described in SECURITY.md §1 and fails closed when isolation is unavailable; do not widen it. (Our own rule, not from the cookbook.)
11. Verification. Anything that touches QGIS objects needs a live test in tests/test_*_live.py (runs in the pinned QGIS 4.2.2 image), not only the offline suite. Say in the commit and release notes whether a change was live-tested, CI-only, or hand-tested (§2).

10. Plan first, ledger always, never regress finished work

Added 2026-10-23 after finished work was broken by later changes three times (rc23 router regression, rc40 routing gap, rc46 fragment guard).

1. Plan before behaviour. A feature, a behaviour-changing fix or a test campaign gets a file in docs/plans/ (template and rules: docs/plans/README.md) in the same pull request as its first line of code, and the plan's status log is updated in the same pull request as each later step.
2. Ledger entry in the same pull request. Every fix or feature adds or updates an entry in docs/FEATURE_LEDGER.json: the source files that implement it, the tests that guard it, and an honest hand_verified line. An entry that says not verified also names its hand_row: the hand-test row id that would verify it, or none: <reason> when no person can check it. tests/test_feature_ledger.py fails if a named guard test or source file disappears, so deleting or renaming a guard is a visible decision, not an accident.
3. Run the guards before you push. python tools/ledger_guard.py lists the ledger features your change touches and runs exactly their guard tests. tools/local_ci.sh --offline runs the same ruff and mypy versions CI pins (a different ruff on the PATH let a CI-only failure through on 2026-10-18). Before a stable release or a big change also run python tools/live_results.py --gate (RELEASE=1 tools/local_ci.sh --offline runs it as a failing check): it lists features a live run saw working whose code, or a module they import directly, changed since; each needs its smoke row rerun and rechecked_at set in docs/LIVE_RESULTS.json. Exit 2 means the run's baseline commit is not in the clone (git fetch --unshallow).
4. A regression test comes with every defect fix. The test reproduces the report (the real prompt, the real layer type), fails without the fix and is named in the ledger. Rows of the hand-test sheets are replayed offline by tests/test_rc46_hand_test_fixes.py; add the new row there too.
5. Hand-test claims are specific. Write "CI only" or "hand-tested on rc47, row A5" in the plan, the ledger, the changelog and the README. Do not close an issue without a hand-verified pass or an owner decision (docs/OPEN_ISSUES_STATUS_*.md lists what each open issue needs).
6. Pull request body uses .github/PULL_REQUEST_TEMPLATE.md, including the before/after table and the ledger and plan boxes.
7. After a release, record it the same day: docs/STATUS_DONE_VS_TESTED_2026-10-16.md, docs/IMPLEMENTATION_TRACKER.md, docs/BUG_TRACKER.md for defects found or fixed, and the open-issue status table.