Fix/strict ci remote readiness - #7
Conversation
…ue-based assertions
…h-performance VOI suite
…e economic sanity checker
…ard visualizations
…dard submission pack
…0-Year Horizon)' as complete
…ket Dynamics (10-Year Horizon)'
…_delphi_vertical'
…for track 'gdpe_0047_game_engine_hardening'
…ign AppTests with UI
|
Important Review skippedToo many files! This PR contains 295 files, which is 145 over the limit of 150. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (5)
📒 Files selected for processing (295)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Review Summary by QodoComprehensive Dashboard Expansion with Temporal Evolution, Adversarial Testing, Governance Layer, and JAX Optimization
WalkthroughsDescription**Major Dashboard and Model Expansion: Temporal Evolution, Adversarial Testing, and Governance** • Comprehensive dashboard redesign from "Equity Localization Edition" to "Temporal Evolution Edition" with 9 tabs (up from 4), adding Spatial Equity, Adversarial Red-Teaming, Stakeholder Consensus, Interoperability, and Publication Narrative • Implemented 10-year temporal evolution simulation with annual trajectory visualization and technology cost improvement modeling • Added adversarial stress-testing engine using JAX/Optax for gradient-based policy robustness evaluation • Integrated multi-persona governance layer (Agentic Auditor) with Delphi protocol for stakeholder consensus building • Refactored parameter system from Pydantic to frozen dataclasses with JAX PyTree registration for automatic differentiation support • Enhanced pipeline with temporal modeling (year, is_annual parameters), proof engine for equilibrium diagnostics, and economic sanity checking • Migrated all scripts to structured logging via loguru with defensive import handling for optional dependencies • Added comprehensive test suite: property-based tests (monotonicity, stability), economic invariant verification, temporal dynamics, and multi-page E2E navigation • Implemented HTA export module for standardized JSON/Excel reporting and persona distiller for policy analysis • Simplified Module D (proxy substitution) and Module E (enforcement) to focused, JAX-compatible implementations • Enhanced sensitivity analysis with unified PSA/VOI/Sobol engine and Bayesian prior grounding for calibration • Added citation validation, manuscript data sync/injection, and quality gate refactoring scripts • Fixed JAX compatibility issues by removing static_argnames constraints and relaxing type hints to Any for tracer support Diagramflowchart LR
A["Dashboard<br/>4 tabs"] -->|Redesign| B["Dashboard<br/>9 tabs"]
B -->|Temporal| C["10-year<br/>Evolution"]
B -->|Adversarial| D["JAX/Optax<br/>Red-Teaming"]
B -->|Governance| E["Agentic Auditor<br/>Delphi Protocol"]
F["Pydantic<br/>Parameters"] -->|Refactor| G["Dataclass<br/>PyTrees"]
G -->|JAX Compatible| H["Auto-diff<br/>Support"]
I["Pipeline"] -->|Enhance| J["Temporal<br/>Modeling"]
J -->|Add| K["Proof Engine<br/>Sanity Checker"]
L["Scripts"] -->|Migrate| M["Structured<br/>Logging"]
M -->|Add| N["Defensive<br/>Imports"]
O["Tests"] -->|Expand| P["Property-Based<br/>Tests"]
P -->|Add| Q["Economic<br/>Invariants"]
File Changes1. streamlit_app/app.py
|
Code Review by Qodo
1. Logging import crashes
|
There was a problem hiding this comment.
Pull request overview
This PR tightens “remote readiness” by hardening CI and developer quality gates (uv-based installs, pre-commit, Renovate), while also adding/committing a large amount of internal “conductor/local_only” track material and generated artifacts.
Changes:
- Update CI and workflows to use
uv, add a unifiedquality_gate, add Renovate + pre-commit automation, and adjust remote E2E setup. - Align
gin-sim/requirements.txtwithstreamlit_app/requirements.txt, and update contributor docs/README to use[dev]+ quality gate. - Add new configs/docs (stakeholder personas, evidence register entries, security policy, citation info), and remove committed Hypothesis artifact files.
Reviewed changes
Copilot reviewed 290 out of 563 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/ci.yaml |
Switches CI to uv + quality gate and adds a broader test matrix. |
.github/workflows/remote-e2e.yml |
Updates remote E2E workflow to install via uv and run Playwright via uv run. |
.github/workflows/pre-commit.yml |
Adds pre-commit CI job. |
.github/workflows/renovate.yml |
Adds scheduled/manual Renovate workflow. |
.github/renovate.json5 |
Adds Renovate configuration. |
.github/dependabot.yml |
Removes Dependabot configuration. |
.github/CODEOWNERS |
Sets repository-wide code owner. |
.pre-commit-config.yaml |
Updates Ruff hooks and adds a local Pyright hook. |
README.md |
Updates install instructions to pip install -e .[dev] and points to scripts/quality_gate.py. |
CONTRIBUTING.md |
Tightens formatting/lint/typecheck commands and adds the quality gate step. |
SECURITY.md |
Adds a security policy and handling rules. |
CITATION.cff |
Updates citation metadata (authors/title/abstract/keywords). |
gin-sim/requirements.txt |
Delegates dashboard deps to streamlit_app/requirements.txt. |
gin-sim/README.md |
Documents the requirements delegation rationale. |
configs/stakeholder_personas.yaml |
Adds stakeholder persona definitions and weightings. |
configs/taught_stakeholders_test.yaml |
Adds taught stakeholder persona test config. |
configs/taught_stakeholders_integration.yaml |
Adds taught stakeholder persona integration config. |
configs/jurisdictions/australia.yaml |
Adds new parameter keys (tapering, audit intensity, remoteness, etc.). |
configs/jurisdictions/new_zealand.yaml |
Adds new parameter keys (tapering, audit intensity, remoteness, etc.). |
configs/jurisdictions/uk.yaml |
Adds new parameter keys (tapering, audit intensity, remoteness, etc.). |
configs/jurisdictions/us.yaml |
Adds new parameter keys (tapering, audit intensity, remoteness, etc.). |
configs/jurisdictions/canada.yaml |
Adds new parameter keys (tapering, audit intensity, remoteness, etc.). |
context/jurisdiction_profiles/australia_evidence_register.yaml |
Adds evidence entries for Medicare cost share and audit intensity. |
docs/REFERENCE_VALIDATION_REPORT.md |
Adds a generated reference validation report (shows unresolved key). |
hta_dossier.json |
Adds an HTA dossier export (currently incomplete/invalid JSON). |
conductor/* |
Adds Conductor project context and state files to the repo. |
local_only/conductor/archive/* |
Adds extensive internal track archives under local_only/. |
.hypothesis/examples/* |
Removes committed Hypothesis example artifacts. |
.hypothesis/constants/* |
Removes committed Hypothesis constants (including absolute local paths). |
$null |
Adds an error log file containing absolute local paths (should not be committed). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| A subdirectory or file C:\Users\60217257\.gemini\extensions\conductor\templates\code_styleguides\python.md already exists. | ||
| Error occurred while processing: C:\Users\60217257\.gemini\extensions\conductor\templates\code_styleguides\python.md. | ||
| A subdirectory or file C:\Users\60217257\OneDrive - Victoria University of Wellington - STAFF\Project - 2026.03 - Economics and Genetic Discrimination\conductor\code_styleguides\ already exists. | ||
| Error occurred while processing: C:\Users\60217257\OneDrive - Victoria University of Wellington - STAFF\Project - 2026.03 - Economics and Genetic Discrimination\conductor\code_styleguides\. | ||
| A subdirectory or file if already exists. | ||
| Error occurred while processing: if. | ||
| The filename, directory name, or volume label syntax is incorrect. | ||
| Error occurred while processing: ($?). | ||
| A subdirectory or file { already exists. | ||
| Error occurred while processing: {. | ||
| A subdirectory or file echo already exists. | ||
| Error occurred while processing: echo. | ||
| A subdirectory or file } already exists. | ||
| Error occurred while processing: }. | ||
| A subdirectory or file else already exists. | ||
| Error occurred while processing: else. | ||
| A subdirectory or file { already exists. | ||
| Error occurred while processing: {. | ||
| A subdirectory or file echo already exists. | ||
| Error occurred while processing: echo. | ||
| A subdirectory or file } already exists. | ||
| Error occurred while processing: }. |
| "compliance_rate": 0.0 | ||
| }, | ||
| "clinical": { | ||
| "total_qaly_gains": No newline at end of file |
| **Track ID:** repo_scrub_audit_20260308 | ||
| **Type:** Chore / Refactor | ||
| **Goal:** Cleanse the remote repository of non-essential development artifacts and AI markers while performing a high-rigor audit for performance and scientific improvements. | ||
|
|
||
| ## 1. Overview | ||
| This track prepares the repository for public/reviewer visibility. It involves moving all "behind-the-scenes" context (Conductor tracks, decision logs, manuscript drafts) to a local-only environment and scrubbing the remote-facing code of any evidence of AI agent usage. Simultaneously, a comprehensive audit will be performed to identify optimization and rigor-alignment opportunities. |
| @@ -0,0 +1 @@ | |||
| {"last_successful_step": "2.5_workflow"} No newline at end of file | |||
| args: [src, streamlit_app, tests, gin-sim, scripts, noxfile.py] | ||
| - id: ruff-format | ||
| args: [--check, src, streamlit_app, tests, gin-sim, scripts, noxfile.py] |
| - name: Run Quality Gate | ||
| run: uv run python scripts/quality_gate.py | ||
|
|
||
| - name: Upload coverage.xml | ||
| uses: actions/upload-artifact@v4 | ||
| if: always() | ||
| with: | ||
| name: coverage-${{ matrix.python-version }} | ||
| path: coverage.xml |
| - `key` | ||
|
|
||
| ## Internal-only allowed keys | ||
| - `Authors' analysis based on policy documents` |
| authors: | ||
| - family-names: "Mordaunt" | ||
| given-names: "Dylan A" | ||
| orcid: "https://orcid.org/0000-0000-0000-0000" |
| ## Phase 2: Relocation & CI/CD Decoupling | ||
| - [x] Task: Categorize and Move Artifacts | ||
| - [ ] Move `conductor/` and process-heavy `context/` files (Decision Log, journal) to `local_only/`. |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on enhancing the project's operational maturity and preparing it for robust remote deployment and external review. It refines the continuous integration pipeline with stricter code quality checks and automated dependency management. A comprehensive documentation framework has been established, detailing the project's development processes and technical foundations, while also organizing internal development artifacts. Additionally, the model's parameterization capabilities have been expanded to support more detailed and localized policy analysis across various jurisdictions. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Ignored Files
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f16a10991d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| target_policy = STANDARD_POLICIES[selected_policy_id] | ||
| history_bundle = simulate_evolution(params_obj, target_policy) |
There was a problem hiding this comment.
Apply taper slider to temporal moratorium projection
When the user clicks 📈 Project 10-Year Trajectory, the code pulls STANDARD_POLICIES[selected_policy_id] directly, so a moratorium run ignores the sidebar taper_range_val customization that is applied in evaluate_cached. This makes the trajectory output inconsistent with the single-year evaluation for the same inputs and can mislead policy comparisons whenever the taper slider is changed from its default.
Useful? React with 👍 / 👎.
| return { | ||
| "uptake_delta": float(reform_res.testing_uptake - base_res.testing_uptake), | ||
| "welfare_change": float(reform_res.welfare_impact - base_res.welfare_impact), | ||
| "uptake_delta": 0.0, | ||
| "welfare_delta": 0.0, | ||
| "compliance_delta": 0.0, | ||
| "welfare_change": 0.0, |
There was a problem hiding this comment.
Compute parameter-mode deltas instead of returning zeros
In the non-dictionary branch, compare_policies now returns hardcoded zero deltas after evaluating only the baseline policy, so any caller that passes ModelParameters gets a false “no difference” result regardless of the reform policy. Because this silently masks policy effects rather than raising an error, it can corrupt downstream analysis if this helper is used in parameter mode.
Useful? React with 👍 / 👎.
| def setup_logging(level: str = "INFO", log_file: str = "outputs/logs/run.log"): | ||
| """ | ||
| Configure loguru with research-grade formatting and optional file sink. | ||
| Configures loguru logging with a standard research-grade format. | ||
| """ | ||
| # Remove default handler | ||
| logger.remove() | ||
|
|
||
| # 1. Standard Console Sink (Colorized) | ||
| format_str = ( | ||
| "<green>{time:YYYY-MM-DD HH:mm:ss}</green> | " | ||
| "<level>{level: <8}</level> | " | ||
| "<cyan>{name}</cyan>:<cyan>{function}</cyan>:<cyan>{line}</cyan> - " | ||
| "<level>{message}</level>" | ||
| # Console handler | ||
| logger.add( | ||
| sys.stderr, | ||
| format="<green>{time:YYYY-MM-DD HH:mm:ss}</green> | <level>{level: <8}</level> | <cyan>{name}</cyan>:<cyan>{function}</cyan>:<cyan>{line}</cyan> - <level>{message}</level>", | ||
| level=level, | ||
| ) | ||
|
|
||
| logger.add(sys.stderr, format=format_str, level=level) | ||
| # File handler | ||
| logger.add( | ||
| log_file, | ||
| format="{time:YYYY-MM-DD HH:mm:ss} | {level: <8} | {name}:{function}:{line} - {message}", | ||
| level=level, | ||
| rotation="10 MB", | ||
| ) | ||
|
|
||
| # 2. File Sink (Rotated, for auditability) | ||
| if log_to_file: | ||
| log_dir = Path("outputs/logs") | ||
| log_dir.mkdir(parents=True, exist_ok=True) | ||
| return logger | ||
|
|
||
| logger.add( | ||
| log_dir / "pipeline_{time:YYYYMMDD}.log", | ||
| rotation="10 MB", | ||
| retention="1 month", | ||
| compression="zip", | ||
| format=format_str, | ||
| level="DEBUG", | ||
| ) | ||
|
|
||
| logger.info(f"Logging initialized at level: {level}") | ||
| return logger | ||
| # Default setup | ||
| setup_logging() |
There was a problem hiding this comment.
1. Logging import crashes 🐞 Bug ⛯ Reliability
src/utils/logging_config.py calls setup_logging() at import time and always adds a file sink to outputs/logs/run.log without creating parent directories, which can raise FileNotFoundError during imports in clean environments/CI. This breaks any codepath that imports logging_config directly or indirectly (e.g., src/utils/path_resolver.py and many scripts).
Agent Prompt
### Issue description
`src/utils/logging_config.py` executes `setup_logging()` at import time and always adds a file sink at `outputs/logs/run.log` without creating the parent directory. In clean CI checkouts/fresh environments, this can raise during module import and break many scripts/tests.
### Issue Context
`outputs/` is gitignored and may not exist. `src/utils/path_resolver.py` imports `logger` from `logging_config`, widening the blast radius.
### Fix Focus Areas
- src/utils/logging_config.py[6-32]
- src/utils/path_resolver.py[6-13]
### Suggested fix
- Remove the module-level `setup_logging()` call.
- In `setup_logging`, create `Path(log_file).parent.mkdir(parents=True, exist_ok=True)` before `logger.add(log_file, ...)`.
- Consider making file logging optional (e.g., `log_file: str | None = None`) to avoid filesystem writes in libraries/tests.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| [tool.pytest.ini_options] | ||
| testpaths = ["tests"] | ||
| addopts = "-v --cov=src --cov-report=xml" | ||
| addopts = "-v --strict-config --strict-markers --cov=src --cov-report=xml --cov-fail-under=45" | ||
| filterwarnings = ["ignore::DeprecationWarning"] | ||
| markers = [ | ||
| "slow: tests that hit deployed services or use slower browser/network interactions", | ||
| ] |
There was a problem hiding this comment.
2. E2e coverage gate failure 🐞 Bug ✓ Correctness
pyproject.toml enables global pytest coverage gating (--cov=src and --cov-fail-under=45) for all pytest runs, but the browser/remote E2E tests run in workflows execute the Streamlit app in a separate process and do not import/execute src within the pytest process. As a result, these E2E jobs will report ~0% src coverage and fail the coverage threshold.
Agent Prompt
### Issue description
Global pytest config enforces `--cov=src` and `--cov-fail-under=45` for every pytest invocation. Browser/remote E2E tests (Playwright) don’t execute `src/` in the pytest process (the app runs in a separate process), so these jobs will fail the coverage threshold.
### Issue Context
This affects at least:
- `local-browser-smoke` in `.github/workflows/e2e-tests.yml`
- `.github/workflows/remote-e2e.yml`
### Fix Focus Areas
- pyproject.toml[91-97]
- .github/workflows/e2e-tests.yml[83-88]
- .github/workflows/remote-e2e.yml[30-37]
### Suggested fix options
1) Add `--no-cov` to the E2E pytest commands in the workflows running Playwright-based tests.
2) Alternatively, remove coverage settings from global `addopts` and enforce coverage only in `scripts/quality_gate.py` / CI quality job.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| - name: Install project dependencies | ||
| run: uv sync --locked --all-extras | ||
|
|
||
| - name: Install pre-commit | ||
| run: python -m pip install --upgrade pip pre-commit | ||
|
|
||
| - name: Run pre-commit | ||
| run: pre-commit run --all-files |
There was a problem hiding this comment.
3. Pre-commit workflow breaks 🐞 Bug ⛯ Reliability
The new pre-commit workflow runs pre-commit run --all-files, but the configured local hooks include a red-team hook pointing to scripts/check_red_team.py (missing from the repo) and a BibTeX validator that imports bibtexparser (not declared in pyproject.toml), causing the workflow to fail. Additionally, the BibTeX hook runs with language: system and uses python ..., which will not use the uv-managed environment created by uv sync.
Agent Prompt
### Issue description
The PR introduces a CI workflow that runs `pre-commit run --all-files`, but the configured hooks are not runnable:
- `red-team-check` references `scripts/check_red_team.py`, which is absent.
- `bibtex-validator` imports `bibtexparser`, which is not declared in `pyproject.toml`.
- `bibtex-validator` runs with `language: system` and `entry: python ...`, which won’t use the uv-managed venv created by `uv sync`.
### Issue Context
This will cause the new `.github/workflows/pre-commit.yml` workflow to fail consistently.
### Fix Focus Areas
- .github/workflows/pre-commit.yml[27-34]
- .pre-commit-config.yaml[44-64]
- scripts/validate_bibtex.py[1-15]
- pyproject.toml[1-54]
### Suggested fix
- Decide one:
- Implement/restore `scripts/check_red_team.py`, or remove/disable the `red-team-check` hook.
- Add `bibtexparser` to `dev` (or main) dependencies in `pyproject.toml`.
- Change `bibtex-validator` entry to `uv run python scripts/validate_bibtex.py context/references.bib` (and similarly for other local python hooks) so CI and developers use a consistent environment.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
Code Review
This pull request introduces a wide range of changes aimed at improving CI, documentation, and repository structure for remote readiness. Key additions include Renovate and CODEOWNERS configuration, a security policy, and extensive project documentation under a new conductor directory. The .gitignore and pre-commit hooks are also enhanced.
However, there are several critical issues that need addressing. An unintended file named $null has been added, and hta_dossier.json is not valid JSON. The CITATION.cff file has been updated but now contains a placeholder ORCID and is missing important fields like license. Additionally, there's a minor documentation typo and an opportunity to improve .gitignore by adding the .hypothesis directory.
I am having trouble creating individual review comments. Click here to see my feedback.
$null (1-22)
This file appears to be an error log or unintended script output. Committing a file named $null containing system-specific error messages is highly unusual and adds clutter to the repository. This file should be removed, and the script that generated it should be fixed to prevent this from happening again.
hta_dossier.json (55-56)
This JSON file is invalid. The clinical object has a key total_qaly_gains without a value, and the file ends abruptly. This will cause parsing errors for any tool trying to read this file. Please ensure the JSON is well-formed. For example, it should have a value (like null or a number) and the closing braces for the objects and the file.
"clinical": {
"total_qaly_gains": null
}
}
}
CITATION.cff (6)
The author's ORCID has been replaced with a placeholder (0000-0000-0000-0000). The previous version contained what appeared to be a valid ORCID. Using a placeholder will break citation generation and proper author attribution. Please restore the correct ORCID.
orcid: "https://orcid.org/0000-0002-9775-0603"
CITATION.cff (26-27)
The license and repository fields have been removed. These are important metadata for software citation and discoverability. The license field is particularly critical for users to understand how they can use the software. Please consider re-adding them.
.gitignore (55)
While this PR correctly removes cached files from the .hypothesis/ directory, the directory itself should be added to .gitignore to prevent these files from being accidentally committed in the future. The .hypothesis directory is used by the Hypothesis testing library to store its database of failing examples, and these are not meant to be version-controlled.
.tmp_streamlit_deploy_check/
.hypothesis/
conductor/product-guidelines.md (67)
There appears to be a typo in the list numbering. The list item "Shareable Links" is numbered as 5. but it should be 7. to follow the sequence.
7. **Shareable Links:** Encode parameter state in URL for collaboration
No description provided.