pega-datascientist-tools

Imported from pegasystems/pega-datascientist-tools (AGENTS.md). Install upstream with npx skills add pegasystems/pega-datascientist-tools. Copyright stays with the author.

Agent Guide for pega-datascientist-tools

This repo is a Python library and tooling suite for Pega data science. Primary code lives under python/pdstools, with tests in python/tests.

Keep edits focused, run the narrowest tests you can, and prefer uv for dependencies and execution.

Critical rules

• Open source repo. Never include customer names, customer data, or internal project names anywhere (code, comments, commit messages, tests).
• Never push without repo-local pre-commit installed and run. Before pushing any branch, install hooks in that checkout with uv run pre-commit install and run uv run pre-commit run --all-files using this repo's .pre-commit-config.yaml. If pre-commit is not installed yet, stop and prompt the user to allow installing the dev tooling (uv sync --extra dev) rather than pushing unhooked.
• Git workflow. Do not git commit on the user's working branch. Stage with git add and let the user commit. Exception: branches you created yourself, or when the user explicitly asks for a commit/PR.
• PR creation auth workaround. If GitHub MCP or the VS Code PR tool fails with Enterprise Managed User authorization errors after a branch has already been pushed, check gh auth status for a non-EMU account that can access the repo. Use that local gh account to create the PR, then switch gh back to the user's preferred account.
• Stay focused, but fix tightly-coupled bugs. Don't fix pre-existing issues unrelated to the task at hand. However, if a refactor surfaces a bug that is directly caused by or tightly coupled to the code you're changing (e.g. a wrong return-type annotation that callers were defensively working around, a stale # type: ignore that hid a real mismatch), fix it in the same PR and call it out in the commit message. Splitting it off creates a regression window where the fix lacks the surrounding context.
• Product naming. User-facing text: "Decision Analysis Tool", "ADM Health Check". Code/CLI: decision_analyzer, adm_healthcheck.
• verbose parameters. Reserve verbose for genuine user-facing progress — tqdm progress bars on long-running I/O, multi-stage CLI output ("Step 1/3: writing parquet…"). For anything that prints internal technical detail (subprocess output, decoded values, "file not found in dir X", schema warnings), drop the parameter and use logger.debug() / logger.info() instead. Never overload verbose to control non-output behaviour (e.g. cache invalidation, retry policy) — that's a separate parameter.
• show parameters on plot methods are an anti-pattern. Plot methods must only build and return the figure; the caller decides how to display it. Adding show: bool = True that calls fig.show() internally causes Jupyter to render the figure twice (once from fig.show(), once from the cell's implicit display of the return value). Never add show to a plot method. For non-Plotly objects that Jupyter cannot auto-render (e.g. pydot.Graph), unconditionally call display() when IPython is available, without gating on a show parameter.
• Plotly label/title clipping checklist. Always do a visual check on exported HTML; Plotly's default margins are tight and common causes of clipping are:
  • Long y-axis tick labels on horizontal bar charts → add yaxis_automargin=True to update_layout().
  • Left y-axis title (rotated text) cut off → margin=dict(l=90).
  • Right secondary y-axis title cut off → margin=dict(r=120).
  • updatemenus button bar overlapping the chart title → lower the button y anchor (e.g. 1.3 → 1.15). These are layout defaults that belong in the library method, not caller-side .update_layout() patches.
• Prefer Polars over Pandas for all data processing.

Quick orientation

• python/pdstools: library code (polars-based data tooling, reports).
• python/tests: pytest suite.
• python/docs: Sphinx docs (with a Makefile).
• reports/*.qmd: Quarto report templates.
• examples/: notebooks and example content.

Setup (local)

Use uv for env management, mirroring CI.

uv sync --extra tests

Optional extras by area:

• Healthcheck + reports: uv sync --extra healthcheck --extra tests
• Docs: uv sync --extra docs --extra all
• Pre-commit hooks: uv sync --extra dev

One-time hook install for any checkout that may be pushed:

uv run pre-commit install

Build / Lint / Test commands

Linting and formatting

Preferred: run the pre-commit hooks (uses ruff + ruff-format + nb-clean).

uv run pre-commit run --all-files

Before pushing a branch, this is mandatory even if you already ran narrower checks while iterating.

If you want to run ruff directly:

uv run ruff check ./python
uv run ruff format ./python

Tests

The full suite takes ~10–15 minutes locally. Prefer narrow, targeted runs (single file, single function, or -k keyword filter) while iterating. Only run the full suite when you're done with a logical chunk of work or when you need broad confidence (e.g. before pushing a PR). CI re-runs everything, so a final full local run is usually enough.

Full test suite (CI default, with coverage; skips heavy tests):

uv run pytest \
  --cov=./python/pdstools \
  --cov-report=xml \
  --cov-config=./python/tests/.coveragerc \
  --ignore=python/tests/healthcheck/test_healthcheck.py \
  --ignore=python/tests/explanations/test_explanations_report.py \
  --ignore=python/tests/healthcheck/test_batch_healthcheck.py \
  --ignore=python/tests/explanations \
  -n auto

Run all tests locally:

uv run pytest python/tests

Run a single test file:

uv run pytest python/tests/healthcheck/test_Reports.py

Run a single test function:

uv run pytest python/tests/healthcheck/test_Reports.py::test_html_deduplication

Run by keyword:

uv run pytest python/tests -k "healthcheck"

Healthcheck tests (requires Quarto + Pandoc):

uv run pytest python/tests/healthcheck/test_healthcheck.py

Docs

Docs live in python/docs and use Sphinx.

cd python/docs && make html

Exporting notebooks to HTML (with plots)

Problem: system jupyter / nbconvert (e.g. from Homebrew) uses a different Python than the project venv. The kernel spawned during --execute won't have pdstools installed and will either fail with AttributeError or die immediately with "Kernel died before replying to kernel_info".

Solution — one-time setup (only needed when the venv is freshly created):

# 1. Install nbconvert into the project venv
uv add --dev nbconvert

# 2. Fix the venv kernel spec to use the absolute venv Python path
python_path=$(realpath .venv/bin/python)
cat > .venv/share/jupyter/kernels/python3/kernel.json << EOF
{
 "argv": ["$python_path", "-m", "ipykernel_launcher", "-f", "{connection_file}"],
 "display_name": "Python 3 (ipykernel)",
 "language": "python",
 "metadata": {"debugger": true}
}
EOF

Exporting (use .venv/bin/python -m nbconvert, never the system jupyter):

.venv/bin/python -m nbconvert --to html --execute \
  examples/articles/AGBExplained.ipynb \
  --output AGBExplained.html --output-dir . \
  --ExecutePreprocessor.timeout=300

Package build (release workflow)

python -m build --sdist --wheel --outdir dist/ .

Code style guidelines

Imports

Ordering, wildcards and placement are linted; use # noqa: F401 for intentional re-exports.

• Optional dependencies: use lazy imports inside the method that needs them (see local_model_utils.py for the pattern). Do not use module-level try/except ImportError blocks. For sub-namespace classes whose methods depend on optional packages (plotting, ML extras, cloud SDKs), extend LazyNamespace (pdstools.utils.namespaces) and declare dependencies = ["..."] + dependency_group = "<extras-group>". It wraps every public method with a dependency check that raises MissingDependenciesException with a friendly install hint, and attribute access on missing methods triggers the same check — no per-class missing-dep stand-in needed.

Formatting

Do not hand-format; ruff-format owns this. Use trailing commas in multi-line literals and call arguments so the formatter keeps them exploded.

Docstring style: numpy

• Use numpy-style docstrings for all public APIs (and any non-trivial internal helper). Sphinx is configured with the numpydoc / sphinx.ext.napoleon toolchain and the rest of pdstools is written this way; a Google-style (Args: / Returns:) or Sphinx-RST (:param x:) block in the middle of an otherwise numpy-style module is a stylistic break and renders inconsistently in the docs.

• Section syntax is linted; the shape to aim for is:

  def foo(x: int, y: str = "a") -> bool:
      """One-line summary in the imperative.
  
      Optional longer description.
  
      Parameters
      ----------
      x : int
          What x is.
      y : str, default "a"
          What y is.
  
      Returns
      -------
      bool
          What the return value means.
  
      Raises
      ------
      ValueError
          When x is negative.
  
      Examples
      --------
      >>> foo(1)
      True
      """

• Reviewers should flag Google/RST-style blocks in PRs and ask for a conversion before merge — left in place they spread by example.

• For AutoAPI-documented classes, document attributes at the declaration site. If a field / accessor / namespace attribute already exists as a real class attribute (plot, aggregates, dataclass fields, lazily-set typed attributes, etc.), put its description immediately after the declaration:

  plot: Plots
  """Plot accessor for visualization methods."""

  This renders cleanly in the generated docs and avoids duplicate object warnings from repeating the same attribute in a class-level Attributes block.

• Don't use class docstrings as method inventories. If a class-level Notes block only restates what from_pdc(), get_df(), plot.foo(), etc. already document individually, delete the inventory. Keep class-level prose for cross-cutting concepts only (for example, sample-vs-full-data semantics or lifecycle/instantiation guidance that spans multiple methods).

• When removing a class-level inventory, preserve any unique information. If the old class docstring contained examples, parameter semantics, return shape details, or caveats that were not already present on the method, move that content onto the relevant method docstring instead of dropping it.

Types and typing

• Use type hints for public APIs and complex internal functions.
• Reuse aliases from python/pdstools/utils/types.py where suitable.
• Reuse the shared frame TypeVar. For helpers that accept and return the same polars frame type, import F from pdstools.utils.cdh_utils._common (F = TypeVar("F", pl.DataFrame, pl.LazyFrame)) rather than declaring a local one. Don't reach for @overload to express "same type in, same type out" — that's what the constrained TypeVar is for. And don't make a helper generic that only one frame type ever reaches; annotate it concretely.

Enum vs Literal vs module constants

Default to Literal for public parameters that accept a fixed set of strings, and plain module-level constants for column names and sentinel values. Reserve Enum for external vocabularies that need bidirectional mapping, iteration, or attached behaviour (see pdstools.infinity).

An Enum used purely as a namespace for string constants is a net loss: every use site pays .value noise, name and value drift apart silently (an Enum whose name is MISSING and value is "missing" will match nothing if the data holds "MISSING"), and it buys no validation against the actual schema. If you need to validate a Literal argument at runtime, write a small validate_*() helper that raises ValueError with the allowed values.

# type: ignore is a smell, not a tool

Treat # type: ignore as a last resort, and treat existing ones as suspicious during any refactor — they often hide real bugs.

• Drop redundant explicit annotations first. Most # type: ignore[assignment] on lines like df: pl.DataFrame = lf.collect() are caused by the redundant annotation itself (polars overloads return the right type already). Remove the annotation; the ignore disappears with it.
• Use cast over # type: ignore[arg-type] for genuine narrowing. cast(list[str], scope_config["group_cols"]) documents intent; # type: ignore[arg-type] hides it.
• Class-level annotations for lazily-set attributes. When an attribute is assigned from a method (not __init__), declare it at class level (_num_sample_interactions: int) instead of writing # type: ignore[attr-defined] at every access site.
• # type: ignore[return-value] almost always means the signature lies. If you have to suppress a return mismatch, the real fix is to update the function's annotated return type to match what it actually returns. Callers downstream are probably defensively working around the wrong type.
• Reserve # type: ignore for genuinely external problems (third-party stubs missing, library bugs) and add a comment explaining why.

Naming conventions

• Functions/variables: snake_case.
• Classes: PascalCase.
• Constants: UPPER_SNAKE_CASE.
• Private helpers: prefix with _.
• Don't leak external naming into Python APIs. When a downstream system uses a different naming scheme (e.g. Pega's pyCreatedBy), keep the Python-facing field name Pythonic (created_by) and handle the translation in the serialization layer — use Pydantic serialization_alias / alias, a custom encoder, or a to_json() method.
• Don't repeat the namespace in its method names. A method reached through a namespace attribute is already qualified by it, so the prefix is pure noise: dm.plot.predictor_binning(), never dm.plot.plot_predictor_binning(). Same for generate, aggregates, agb.
• No get_ prefix. If it takes no arguments, make it a property (or cached_property when the work is non-trivial). If it does take arguments, name it for what it returns (predictor_contributions(...), not get_predictor_contributions(...)).
• Namespace classes that return aggregations are plural (Aggregates, Plots), matching the accessor they're bound to.

Error handling

• Raise specific exceptions; provide actionable messages.
• For optional dependencies, use MissingDependenciesException patterns.
• Prefer ValueError/TypeError for argument validation.
• Use pytest.skip for environment-dependent tests (e.g., Quarto).
• Package-manager neutral messages: do not hardcode pip install or uv pip install in error text. Name the missing package and let the user choose their tool.

Logging

• Log at debug/info levels. The module-logger and no-print rules are linted; # noqa: T201 with a reason is the escape hatch for genuinely user-facing CLI output.
• debug as a parameter name: only when it changes the return value (extra columns, etc.), not for controlling log output.

Data and polars

• Use polars.LazyFrame where feasible; collect only at boundaries.
• Prefer expression-based transforms (pl.Expr) over Python loops.
• Keep IO in dedicated helpers; avoid side effects in pure functions.
• Column names in DecisionAnalyzer use human-friendly display names (e.g. "Issue", "Action", "Interaction ID") defined in table_definition.py. Never use internal Pega names (pyIssue, pxInteractionID, etc.) in new code. Internal working columns (pxRank, is_mandatory, day) are exceptions.

Polars gotchas worth memorising

• filter(~bool_expr) and nulls. ~null evaluates to null, and filter treats null the same as False — the row is dropped. If the boolean expression you're negating can produce nulls (e.g. a comparison against a column that may be null), explicitly .fill_null(False) before negating, or rewrite using is_null() | (...) so the predicate is total. Bit us in the AGB-empty-context fix (#667).
• agg(...).any(ignore_nulls=False). Default any ignores nulls, which silently turns "we have no data" into False. When the distinction matters (e.g. tri-state usesNBAD / usesAGB flags that need to render as ?), pass ignore_nulls=False and let the null propagate.

Filter sentinel rows at the IO boundary

External systems (Pega, third-party exports) sometimes ship "totals" or "summary" rows mixed into the same table as real instances — e.g. AGB models emit a per-configuration row with empty context keys that duplicates aggregate data and can carry a miscomputed AUC. Drop these in the _validate_*_data helpers, not in the report layer. That way every consumer (Quarto reports, Streamlit pages, scripts) sees a consistent view, and nobody has to remember to re-apply the filter.

When the discriminator (e.g. ModelTechnique == "GradientBoost") can be null because the column wasn't always emitted in older sources, only filter when the discriminator is known. Falling through on unknown values surfaces genuine data-quality issues instead of hiding them.

Tests

• Use pytest fixtures for shared setup.
• Keep tests deterministic and data-driven.
• Do not download or execute third-party ML models in unit or UI smoke tests. Tests should use deterministic synthetic arrays and mocks to verify pdstools' orchestration, caching, input/output handling, and error paths; they must not validate vendor model quality, algorithm internals, network access, or model-cache availability.
• Real model or external-algorithm integration tests are opt-in only: isolate them in explicitly marked slow/integration tests, keep them out of the default test path, and never make ordinary CI depend on remote model downloads.
• Mark slow tests with @pytest.mark.slow.
• Use pytest.skip for missing external tools (Quarto, Pandoc).
• Minimum coverage: 80 % for new and overall code (CI-enforced).
• Strongly prefer exact-value assertions over structural checks. Tests that only verify column names, non-empty results, or height > 0 give false confidence — they pass even when computations are wrong. Use small, minimal datasets (see data/da/sample_eev2_minimal.csv) where every expected value can be traced back to the input data. Structural/smoke tests on larger datasets are fine as a complement, but the correctness backbone should be exact-value tests.
• Drop # pragma: no cover when you add tests. Pragmas are for code that genuinely can't be exercised (platform-specific branches, defensive raise after exhaustive if/elif). Once a method has a test, the pragma is stale and misleading — remove it in the same PR that adds the test.

Notebooks and reports

• Notebooks for docs should be empty/not pre-run.
• Quarto reports live in reports/*.qmd; keep params in YAML helpers.
• When changing reports, check size reduction behavior and Plotly embedding.
• Editing .ipynb notebook files directly is fine — modern tooling handles the JSON format well.

Streamlit apps

Core principle: zero-functionality presentation layers

All functionality lives in the library. Streamlit apps are zero-functionality presentation layers. Every calculation, transformation, formula, and data shape an app exposes must be reproducible in a Jupyter notebook or a short script using only pdstools.<module> imports. Apps add accessibility (UI, no coding required); they never add capability.

Concretely:

• No business logic in Home.py or pages/*.py. Pages compose widgets, read session state, and call into the library. If a page contains a non-trivial calculation, a custom data transform, or a domain formula, lift it into the relevant library module (pdstools.adm, pdstools.ih, pdstools.impact_analyzer, etc.) and call it from the page.
• Plot construction lives in the library (<module>/Plots.py or a plot namespace on the analyzer). Pages call those functions and may tweak layout via .update_layout() before st.plotly_chart(fig).
• Excel/CSV/PDF export builders are library code. A page calls analyzer.generate.<format>(...) and hands the bytes to st.download_button. No build_excel-style functions inside app/.
• Formulas worth showing in the UI need a first-class library representation (e.g. a Formula dataclass with .expression, .filled(values), .evaluate(values)). The same object then drives both the calculation and any Markdown/LaTeX rendering — UI and notebook share one source of truth.
• Examples directory should not ship parallel Streamlit apps. examples/<feature>/ is for notebooks and short scripts that demonstrate the library API. The matching app belongs in python/pdstools/app/<feature>/.

The smell test: "Can I do exactly the same thing in a Jupyter notebook with no streamlit import?" If the answer is no, the functionality is misplaced — move it into the library and have the page call it.

Architecture

• Apps live under python/pdstools/app/<app_name>/ with a Home.py entry point and numbered pages in a pages/ subdirectory.
• Three apps exist: health_check, decision_analyzer, impact_analyzer. All are launched via the unified CLI in python/pdstools/cli.py.
• Shared utilities go in python/pdstools/utils/streamlit_utils.py. App-specific helpers (data loading, custom widgets) go in a co-located <app>_streamlit_utils.py (e.g. da_streamlit_utils.py).

Page boilerplate

Every page must start with two calls before any other Streamlit output:

from pdstools.utils.streamlit_utils import standard_page_config
standard_page_config(page_title="Page Title · App Name")

Home pages additionally call show_sidebar_branding(title) to set the sidebar logo and title (sub-pages re-apply it automatically).

Session state conventions

• Store the main data object under a well-known key (e.g. st.session_state.decision_data).
• Guard every sub-page with an ensure_data() call that shows a warning and calls st.stop() when data is missing.
• Use the _persist_widget_value pattern (copy widget value to a second key on on_change) to survive Streamlit's page-navigation state clearing. Widget keys use a _ prefix; persisted keys omit it.
• Prefix filter session-state keys with a filter_type string ("global", "local") so independent filter sets don't collide.

Caching

• Use @st.cache_resource for stateful / non-serializable objects (e.g. DecisionAnalyzer instances). Prefix unhashable parameters with _ and supply a data_fingerprint string for cache busting.
• Use @st.cache_data for pure data transforms and plot functions. When inputs include pl.LazyFrame or pl.Expr, pass a custom hash_funcs dict (see polars_lazyframe_hashing in da_streamlit_utils.py).

CLI integration

• CLI flags (--deploy-env, --data-path, --sample, --temp-dir) are propagated to the Streamlit process as PDSTOOLS_* environment variables. Read them via helpers in streamlit_utils.py (get_deploy_env(), get_data_path(), etc.), never via os.environ directly in page code.

Plotly charts

• All charts use Plotly. Display with st.plotly_chart(fig) — do NOT pass use_container_width (deprecated) or width="stretch" (default). Use config= for Plotly config options.
• Keep plot construction in the library layer (plots.py); Streamlit pages call those functions and may tweak layout via .update_layout() before rendering.

Shared About page

• Use show_about_page() from streamlit_utils.py for a standardised About page. A page file can be as small as two lines (page config + show_about_page()).

Testing widget interactions, not just initial renders

streamlit.testing.v1.AppTest is the right tool for both "page renders" smoke tests and state-transition tests. The v5.0.0 launcher shipped with two P1 upload-flow regressions that the existing AppTest suite happily green-lit, because it only covered initial renders:

• HC: switching the data-source dropdown to "Direct file upload" rendered no uploaders at all.
• DA: dropping a file into the uploader after the sample auto-load did nothing — analyzer kept pointing at the autoloaded data.

Both are pure session-state interactions (an on_change callback deletes a key, the next-run guard then trips). They're invisible to sub-page tests that pre-seed st.session_state["dm"] / st.session_state["decision_data"] to bypass the home page entirely.

Rule: any widget whose interaction mutates session state in a non-trivial way — on_change callbacks, buttons that store results, sliders/selectboxes that drive subsequent renders — needs a state-transition AppTest. This applies to home pages and sub-pages.

Pattern for a selectbox / slider:

at = AppTest.from_file(str(home_py)).run()
at.selectbox(key="data_source").set_value("Direct file upload").run()
assert len(at.file_uploader) >= 1

Pattern for a button that stores results:

gen_button = next(b for b in at.button if b.label == "Generate Health Check")
gen_button.click().run()
assert any("Health Check" in getattr(b, "label", "") for b in at.get("download_button"))
assert "file" in at.session_state["run"][at.session_state["runID"]]

Reference implementations:

• Home-page upload flow: test_upload_replaces_autoload.py, test_direct_upload_renders.py
• Sub-page sliders: test_threshold_sliders.py
• Sub-page selectbox: test_arbitration_scope_selectbox.py
• Button → download flow: test_generate_button.py
• Multiselect filter pipeline: test_data_filters.py

If you change a widget's key, its on_change handler, or any session- state key it reads/writes, add or update the matching state-transition test in the same PR.

General Streamlit rules

• Never use st.experimental_* APIs — they have been removed. Use the stable equivalents (st.cache_data, st.cache_resource, etc.).
• Avoid components.html() with custom JavaScript. It runs in an iframe, is fragile, hard to debug, and does not integrate with Streamlit's state management (session state, theming, reruns). Prefer native Streamlit widgets and layout primitives. Only resort to custom JS when there is genuinely no Streamlit-native alternative.
• Use Streamlit magic (bare strings/expressions) for static markdown where convenient, but prefer explicit st.markdown() / st.write() for dynamic content.
• Keep heavy computation out of page scripts; delegate to cached functions or the library layer.

Checking private customer data for ADM Health Check caches

When asked to scan a private customer-data folder for newer or missing ADM Health Check cache candidates, keep the work read-only until the user explicitly approves conversion or replacement. These folders can contain private customer files, so do not write customer names, source paths, filenames, row samples, or generated reports into tracked repo files. Use temporary scripts under /tmp for any helper code and remove them when done.

Workflow:

• Inventory existing top-level HC folders and canonical cache files: PR_DATA_DM_ADMMART_MDL_FACT.parquet, PR_DATA_DM_ADMMART_PRED.parquet, and optional PR_DATA_DM_SNAPSHOTS.parquet.
• Search candidate source files by role using filenames such as model snapshot, predictor binning snapshot, prediction snapshot, PR_DATA_DM_ADMMART_MDL_FACT, PR_DATA_DM_ADMMART_PRED, and PR_DATA_DM_SNAPSHOTS. Exclude existing HC output folders from the source scan.
• Summarize candidates by customer folder and source subfolder before reading data content. Use metadata first: role coverage, file counts, modified dates, and sizes.
• Validate only shortlisted candidates through the same library path the app uses: import_health_check_data(...). Do not call save_health_check_parquet(...) during the recommendation phase.
• Compare validated row counts against any existing canonical HC parquet row counts. Treat newer dumps with much smaller model counts, missing predictor data, parse errors, or selected/filtered naming as "do not replace blindly" candidates.
• In the chat report, keep it concise: client name, folder name, row counts when validated, and the suggested action. Clearly separate "create HC", "replace existing HC", "needs manual/import-option pass", and "skip". Do not modify or replace private data until the user confirms the recommendation.

Design principles for new functionality

These principles apply when adding new features, modules, or classes — whether in a PR or an agentic edit.

Extend before you create

Before adding a new class or module, search the codebase for existing models or utilities that already cover similar ground. Add methods, fields, or class-methods to the existing code rather than building a parallel hierarchy. Duplicate abstractions create maintenance burden and confuse users about which entry point to use.

Earn your abstractions

Don't introduce design patterns (Builder, Factory, Registry, etc.) unless the complexity genuinely demands them. If a Pydantic model gives you validation, defaults, and serialization for free, use its constructor directly. A fluent builder that only assigns fields and calls a constructor adds indirection without benefit.

One module per domain class — no *Utils.py grab-bags

A class belongs in a module named after it (ContextOperations.py, not ExplanationsUtils.py). A *Utils module signals that nobody decided what the code is, and it becomes a magnet: constants, enums, a domain class, and a few loose functions accumulate with no cohesion and no obvious import site.

If you find yourself reaching for Utils, split by what the contents actually are:

• Shared vocabulary (column names, sentinels, Literal aliases and their validators) → a private _constants.py.
• A class with state and behaviour → its own PascalCase.py module.
• Genuinely generic, cross-module helpers → the existing pdstools/utils/ package, in the topical module that fits (cdh_utils, namespaces, types), not a new per-feature one.

One concern per PR

Keep pull requests focused on a single, well-defined change. If a feature touches multiple independent areas (e.g. conversion + security + optimization), split them into separate PRs so each can be reviewed and merged on its own merits.

Right-size your validation and security

Validate inputs and enforce sensible limits (allow-lists, size caps, type checks). But don't add threat mitigations that don't map to a real attack surface — e.g. scanning for SQL injection in values that are never executed as SQL, or HTML-sanitising strings that are never rendered in a browser. Keep security code proportional to the actual risk; otherwise it creates false confidence and false-positive noise.

Prefer thin, valuable wrappers

When wrapping a third-party library, add genuine value: compatibility fixes, sane defaults, validation, or integration with pdstools conventions. Don't wrap standard calls with layers of config objects and result models just for the sake of wrapping — if calling the library directly is clear enough, let users do that.

Keep the Python API Pythonic

Public APIs should feel natural to a Python developer. Translate between external naming conventions (Pega property names, JSON key schemes) at the serialization boundary, not in field names or function signatures. See the naming-conventions section above.

Keep parameter surfaces small

Every parameter on a public function is an API commitment. Only expose what users genuinely need to control. Internal filtering or behavior toggles should be handled close to where they act, not threaded through every layer of a call chain. If a toggle only matters at one point in the pipeline, apply it there — e.g. filter in the data-loading step, or accept a pre-filtered frame — rather than adding it to every function between the caller and that point. Smell: the same parameter name appearing in 3+ functions in the same call chain.

Use Python's built-in defaults

Use keyword arguments with literal defaults (def foo(top_n: int = 20)) — this is idiomatic Python, self-documenting, and immediately visible in IDE tooltips. Avoid centralizing defaults in a config dataclass or defaults singleton that every function references; that adds indirection without benefit for a library. The function signature is the documentation of its defaults. A frozen-dataclass of defaults is an engine/enterprise pattern, not a fit for a lightweight analysis package.

Exception — shared option sets across 2+ public entry points. When the same set of 5+ options recurs across multiple public methods (e.g. report rendering options shared by model_reports and health_check), extract a TypedDict and pass it via **options: Unpack[OptionsType]. This keeps the signatures consistent, documents the shared surface in one place, and prevents drift when a new option is added (one TypedDict update vs. N signature updates). pdstools.adm.Reports.ReportOptions is the reference. Do NOT extract a TypedDict just to shorten a single method's signature — that's indirection without benefit. The trigger is shared across methods, not signature length.

Namespace facade for large analyzer classes

For any class that grows beyond ~20 public methods, split related methods into sub-namespace classes attached as instance attributes. ADMDatamart is the reference: dm.plot, dm.aggregates, dm.agb, dm.generate, dm.bin_aggregator. Each sub-namespace:

• Lives in its own module (adm/Plots.py, adm/Aggregates.py, …).
• Takes a single parent argument named datamart (or the equivalent name used by the parent class) and stores it as self.datamart.
• Uses if TYPE_CHECKING: from .Parent import Parent to avoid circular imports for the back-reference.
• Is instantiated in the parent's __init__ and assigned to a short, dot-completion-friendly attribute (self.plot = Plots(self)).

This keeps the public API discoverable (one class to import; dot completion reveals everything) without ballooning the parent into a multi-thousand-line god class. Use the same pattern when refactoring existing fat classes — don't invent a new convention.

super().__init__() is required in LazyNamespace subclasses. It looks removable — the base __init__ takes no arguments and Python 3 doesn't need it for plain object construction — but LazyNamespace.__init__ sets _dependencies_checked, and without it the optional-dependency machinery breaks at first attribute access. Don't strip it in a cleanup pass.

I/O lives in classmethods, not __init__

Keep __init__ pure: it should only accept already-loaded data structures (typically pl.LazyFrames) and configuration. All file, network, and S3 I/O lives in alternative constructors named from_<source> (e.g. from_ds_export, from_s3, from_dataflow_export, from_pdc). This mirrors the pl.read_csv / pl.scan_csv idiom and makes the class trivially testable with synthesized data — no monkey-patching required.

Inside from_<source> classmethods, delegate path resolution to pdstools.pega_io.File.read_data for anything path-like. It already handles single files, directories (Hive-partitioned layouts), archives (zip / tar / gzip), BytesIO uploads, and glob patterns ("data/**/*.parquet"). If you discover a new input shape that isn't covered, extend read_data rather than rolling a local resolver on the analyzer class — every analyzer benefits and the entry point stays singular. Use the more specialised read_ds_export only when you need its ADM-specific smart-name lookup ("model_data", "predictor_data") or remote-URL fetching.

Validate at the point of use, not ahead of it. Don't write validate_data_folder()-style guards that stat paths, check for expected filenames, and raise before the real read happens. They duplicate logic the reader already has, drift out of sync with it, introduce a TOCTOU gap, and turn one clear FileNotFoundError into two competing error messages. Let the read_data / scan_* call fail, and if its message isn't actionable enough, improve it there — every caller benefits.

The whole pdstools.pega_io module is the single funnel for user-facing path → polars reads (CodeQL py/path-injection is suppressed there at config level on that basis). Inside the funnel, _scan_by_extension is the one place pl.scan_* / pl.read_* is actually called for a leaf path. New format-specific helpers (scan_parquet_path, future scan_csv_path, …) and reader modules (action_analysis.py, future per-format helpers) should delegate to _scan_by_extension rather than calling pl.scan_* themselves — otherwise we end up with parallel "single sources of truth" and the generic CSV/JSON defaults (null values, date parsing, pxResults fallback) drift between callers.

When the source is a cloud service (S3, GCS, Azure Blob), keep the heavy SDK (boto3, google-cloud-storage, …) as a lazy import inside the classmethod and gate it via MissingDependenciesException with a deps_group pointing at the right optional extra (e.g. pega_io). Test the classmethod with moto (or the equivalent mocking library for the provider) added to the tests extra, and factor a small _download_from_<provider> helper so tests can mock at one well-defined seam. Reference: ADMDatamart.from_s3.

The "gold-standard" checklist for top-level analyzer classes

ADMDatamart and DecisionAnalyzer are the reference implementations for any new (or refactored) top-level data-science class. When adding or rewriting one, hold yourself to all of these:

1. Pure __init__ — accepts already-loaded pl.LazyFrames and plain configuration only. No file paths, URLs, S3 keys, network calls, or sub-process invocations.
2. All IO lives in from_<source> classmethods (see the section above). Each one delegates to __init__ after loading.
3. Schema in a dedicated module (Schema.py) using polars schemas; apply via cdh_utils._apply_schema_types(df, Schema.X).
4. _validate_*_data helpers do type coercion, default columns, sentinel-row filtering, and any other normalisation needed so that downstream consumers can assume a clean shape.
5. Namespace facade for >20 public methods — split related methods into sub-classes attached as instance attributes (.plot, .aggregates, .generate, .bin_aggregator). See "Namespace facade for large analyzer classes" above.
6. Optional dependencies lazy-imported inside the method that needs them and gated via MissingDependenciesException(deps_group=...).
7. return_df: bool = False on every public plot method, paired with @overload so type checkers know which return shape applies.
8. Zero # type: ignore in core files. Peripherals may carry one or two with comments explaining the genuinely external problem (third-party stub gap, library bug).
9. Helpers small enough to unit-test individually with synthetic LazyFrames — exact-value assertions, not just structural checks.
10. Logger via logging.getLogger(__name__) at module top; no print() in library code.

When auditing an existing class, walk the list, file plan items for each gap under docs/plans/<feature>/, and tackle them in priority order (typing < tests < refactoring) so the riskiest work lands on the most-tested code.

return_df parameter on plot methods

Every public method that produces a chart should accept return_df: bool = False as a keyword-only argument. When True, return the underlying (Lazy)Frame that drives the chart instead of the figure. This pattern (used consistently across ADMDatamart.plot) makes plots scriptable, testable, and composable without forcing users to re-derive the aggregation. Pair with @overload so type checkers know which return shape applies.

Feature backlog / TODO files

Each feature area maintains a backlog in docs/plans/<feature>/ — one Markdown file per open item, plus a README.md. All four active areas (adm/, decision-analyzer/, health-check/, impact-analyzer/) use this layout. Do not add new single-file *-TODO.md backlogs.

• Adding an item = create docs/plans/<feature>/<slug>.md. No shared state; no merge conflicts with parallel PRs.
• Resolving an item = git rm docs/plans/<feature>/<slug>.md in the PR that resolves it. The deletion is the audit trail; the PR description / commit message captures the what and why.
• Check before working. When starting work on a feature area, read its plan directory first.
• Update as you go. git rm items when they land on master. Add new files when you discover bugs, limitations, or ideas during development.
• Priority levels: P1 = high, P2 = medium, P3 = nice-to-have.
• Filename: short kebab-case slug (lazy-plotly.md, binagg-rename.md). For Streamlit page items, prefix with the page number (page3-topk-limiter.md).
• Contents: title, priority, files touched, problem statement, proposed approach, any cross-refs. Aim for under one screen.

Listing open items across a feature:

ls docs/plans/decision-analyzer/

Finding P1 items across all features:

find docs/plans -name '*.md' | xargs grep -l 'Priority:** P1'

Inline # TODO vs plan-file entries

• Plan file (docs/plans/<feature>/<slug>.md) — substantive backlog items: missing features, multi-line refactors, known limitations, bugs worth tracking, design questions. Anything a future contributor would want to discover by reading the backlog rather than by stumbling onto a comment.
• Inline # TODO — small, code-local hints tied to a specific line: "consider a faster path here", "revisit when polars supports X". Should be self-contained.
• Anchor when both apply. If an inline note has a matching plan entry, link them: # Tracked in docs/plans/<feature>/<slug>.md — <one-liner>
• Don't park lists of TODOs at the top of a file/page. Lift them into the plan directory and replace the block with a single pointer comment.

Surfacing follow-up work

When a conversation or refactor uncovers something that won't be addressed in the current PR, don't let it evaporate. Pick the right venue and either file it or hand the user a draft they can file:

• GitHub issue for anything other contributors should see and pick up: bugs, missing features, design questions, promotion candidates from prototype → product. Agents typically can't gh issue create on this repo (EMU restrictions) — write a ready-to-paste draft (title + body in markdown) and hand it to the user. Keep one issue per concern, not omnibus dumps.
• Plan-file entry (docs/plans/<feature>/<slug>.md) for backlog items scoped to a specific feature area that aren't substantial enough to be their own issue, or that need design work before they can be filed.
• Inline # TODO for code-local hints (see "Inline # TODO vs plan-file entries" below).

Triggers to raise follow-ups proactively:

• A "we should also…" or "while we're here…" thought that's out of scope for the current PR.
• A bug discovered that isn't tightly coupled to the current change (the coupled ones get fixed in-PR — see "Stay focused" in critical rules).
• A # type: ignore, defensive isinstance branch, or # pragma: no cover you can't justify removing in the current PR.
• Repeated workarounds across multiple files that point to a missing abstraction.
• Prototype code that has matured enough to belong in pdstools proper.

Default to nudging the user: "This is out of scope for the current PR — want me to draft an issue?" — better one too many drafts than a silently-dropped follow-up.

Read the whole issue thread before fixing

For any GitHub issue with comments, read the full discussion before designing the fix. The resolution often shifts from the title: #667 started as "filter in the report layer" and converged on "filter in ADMDatamart at load time" by the third comment. Acting on the title alone would have produced a correct-looking but wrongly-placed fix that another reviewer would have to redo.

Verify "removed / renamed / dropped" claims against current source

When writing changelog entries, migration guides, deprecation notes, or any prose that describes an API change as having happened, grep master for the symbol first. PR titles and commit messages routinely overstate the change ("drop the **kwargs catch-all" can mean "replace bare **kwargs with **options: Unpack[ReportOptions]", which keeps the splat working and merely types it). A migration guide that tells users to rewrite call sites that don't actually need rewriting is worse than no entry at all — it forces a noisy review cycle and erodes trust in the rest of the document.

Workflow:

1. Identify the symbol/parameter the PR claims to change.
2. grep for it in python/pdstools/ on current master.
3. If it still exists, read how it's used now and write the entry to match reality (or omit the entry entirely if the change is transparent to callers).
4. Only describe something as "removed" / "renamed" if step 2 confirms the old name is gone.

This applies double when the AGENTS.md design rules contain an explicit exception for the pattern in question (e.g. "shared option sets across 2+ public entry points → use Unpack[TypedDict]"). A grep-then-write check would have caught the bogus "ReportOptions removed" migration entry that prompted this rule.

Don't land a "plan-files-only" PR

Plan files (docs/plans/<feature>/<slug>.md) are designed to be created and resolved alongside code changes — they're audit trail, not standalone deliverables. Avoid landing a PR that only adds plan files. If a sweep produces follow-up work that won't be picked up immediately, embed the context inline in subsequent agent prompts rather than committing dedicated backlog files. The next PR that touches the area can file (and resolve) plan entries as part of its diff.

Contrib and workflow notes

• Main tests are python/tests; CI runs multi-OS and multi-Python.
• Healthcheck tests are separate and require extra deps/tools.
• Use uv to mirror CI; avoid mixing system pip unless necessary.

Tips for agentic changes

• Prefer minimal diffs; avoid touching unrelated files.
• Do not edit generated files (e.g., uv.lock) unless required.
• Keep security in mind; do not commit secrets or local data artifacts.
• When the IDE shows type errors after an edit, distinguish pre-existing from newly introduced. Only fix the new ones silently; ask before touching pre-existing issues.

Parallel sub-agent workflow (git worktrees)

When dispatching multiple background sub-agents that each work on a different branch in parallel, they MUST be isolated via git worktree — otherwise they share the user's working directory, which is a recipe for:

• One agent's git checkout <branch> switching the user's HEAD without warning.
• Agent A's WIP edits showing up in agent B's git status and getting swept into the wrong commit.
• The user's foreground edits getting tangled with agent edits and ending up on the wrong branch.

Setup

Create one worktree per parallel branch under a sibling directory:

mkdir -p ../pdstools-worktrees
git worktree add ../pdstools-worktrees/<branch-name> <branch-name>

Each sub-agent is told to cd ../pdstools-worktrees/<branch-name> first and do all work there (edits, tests, commits, push). The agent prompt should explicitly forbid touching the main checkout.

After git worktree add, run uv sync --extra tests once in the new worktree — the .venv is per-worktree, not shared.

Pre-commit hooks are not installed in new worktrees. The .git/hooks/ directory is per-worktree, so the symlinks created by pre-commit install in the main checkout don't carry over. A sub-agent that just runs git commit in a fresh worktree will bypass the hooks entirely, and the lint failure shows up later in CI.

Required workflow for every push-capable worktree:

• Run uv sync --extra dev if pre-commit is not available yet. If the environment still does not have pre-commit, stop and prompt the user before pushing anything.
• Run uv run pre-commit install once per worktree after uv sync.
• Run uv run pre-commit run --all-files as the final step before any push. CI mirrors this exact repo-local config.

If a batch of agent PRs all fail the lint check at once, the rescue loop is:

for wt in <branch-1> <branch-2> ...; do
  cd ../pdstools-worktrees/$wt
  uv run pre-commit run --all-files || true   # auto-fixes most things
  git diff --quiet || { git add -u && git commit --amend --no-edit && git push -f; }
done

Cleanup

When a branch is merged or abandoned, prune its worktree:

git worktree remove ../pdstools-worktrees/<branch-name>

When to skip worktrees

• A single background sub-agent + the user staying in read-only mode in the main checkout is usually fine.
• Sequential sub-agent runs (one finishes before the next starts) don't need worktrees.
• The moment a second concurrent writer is involved, set up worktrees.

Rescuing a worktree after a session restart

When a session restart kills an agent mid-edit, its worktree is left with uncommitted modifications. Don't blindly discard or commit them. Workflow:

cd ../pdstools-worktrees/<branch-name>
git status                          # what changed?
uv run ruff check <changed-paths>   # does it lint?
uv run pytest <targeted-tests> -q   # do tests still pass?

If clean: commit-as-is, rebase onto current origin/master, push. If broken: git restore . (or cherry-pick the salvageable parts) and re-dispatch the agent. Either way, decide explicitly — leaving a worktree in a half-finished state silently rots and tangles with the next sweep.