Review of the Portfolio Engine Gemini audit pilot¶
1. What it is¶
A local Python controller (run_gemini_pilot_v3.py, about 490 lines, 8 unit tests with mocks). The intended workflow (#318, "Workflow objective clarification"): ChatGPT assigns bounded work on eligible issues; an agent works intermittently without Atul approving each step; results are recorded on GitHub.
Each run:
1. checks the local checkout's origin;
2. makes a fresh full clone of --ref (default main);
3. checks the four batch-* labels exist;
4. finds open batch-ready issues (sorted, capped at 1–10);
5. for each issue:
- re-fetches the issue and its comments;
- captures evidence: the issue, AGENTS.md, CLAUDE.md, and two #315 documents, with hashes;
- model 1 (gemini-3.6-flash, fallback gemini-3.5-flash) writes a checklist of review questions;
- model 2 (gemini-3.5-flash-lite, fallback gemini-2.5-flash-lite) writes the audit using the evidence and the checklist;
- posts the audit as a comment;
- with --apply-labels, moves batch-ready → batch-running → batch-reviewed, or to batch-blocked on error, verifying each change;
6. writes a run record (JSON) with evidence paths, model attempts and the comment URL.
The live run (10 Oct, #317, without --apply-labels) took 52 seconds and posted one report. ChatGPT then posted a correction to that report.
2. What is good (keep it)¶
- Safe by default. Without flags it only does preflight.
--execute-reviewsand--apply-labelsare separate opt-ins. - Fails closed:
- missing labels;
- a missing API key;
- a dirty or mismatched clone;
- an issue whose state changed since discovery.
- Verified label changes: after each change it re-fetches the issue and checks that only one lifecycle label is present.
- Recovers from an unclear post: a run-ID marker in the report lets it search the comments before retrying, so it never posts twice.
- Run records with evidence hashes and model attempts.
- Planner + worker split: a stronger model plans, a cheap model writes. That keeps the cost low (both are Flash-class).
- Tests cover discovery, failing closed, verified transitions and recovery.
- The rules in PE's
AGENTS.md§6 match the code.
3. Problems found¶
| # | Problem | Effect | Severity |
|---|---|---|---|
| P1 | It reviews the wrong code. --ref defaults to main, but the work for #315–#317 is on feature branches |
The #317 audit read main while the issue's work was on feature/issue-313-category-tree |
High |
| P2 | It sends both checkouts to the model. The local checkout (dirty, another branch) and the clean clone are both in the evidence | Gemini mixed them up and said the source was a clean main; ChatGPT had to post a correction |
High |
| P3 | The evidence doesn't include the code. It holds the issue plus four fixed files, two of them specific to #315; no source files and no PR diff | Gemini stated "no N×N correlation module exists in the repository" as a verified observation, but it never saw the code, only the comments | High |
| P4 | Specific to #315. The file list and the regex in detect_decision_conflict were written for #315 |
It doesn't generalise to other issues without code changes | Medium |
| P5 | Agent identity. Comments and labels are made with gh, which is logged in as Atul |
Gemini's audits appear as written by moneymarche-dev; the report header names the models, but there are no Agent:/Role:/Seat: lines |
Medium |
| P6 | Data class. Private-repo content goes to the Gemini API on the free tier (confirmed by Atul, 10 Oct) | Spec §5B allows only public and synthetic data on free tiers, and free-tier content may be used to improve Google's products. Stop sending private-repo content until the key is paid or the content is public or synthetic | High |
| P7 | Subprocesses get no stdin=. |
gh or git can hang on Windows waiting for input; this exact bug was MMDEP's P1 in iteration 1 |
Medium |
| P8 | Clones pile up. Each run makes a full clone and never deletes it | Disk use grows with every run | Low |
| P9 | Cost isn't recorded. No token counts in the run record | The cost per audit is unknown | Low |
| P10 | Versions kept as copies (run_gemini_pilot.py, _v2, _v3, .before_utf8_fix, a zip bundle), outside any repository |
No history and no review; hard to know which version ran | Low |
| P11 | It doesn't loop yet. It runs once per command | The "intermittent, no approval per step" objective isn't reached | Expected for a pilot |
4. Why it was shelved, and what that teaches¶
ChatGPT, the PE lead, designed and debugged the controller itself: v1 to v3 in about a day, with eight triage and design comments on #318. That pulled the lead agent off Portfolio Engine. The code wasn't the problem; the tooling belonged to the wrong role. Under the §5C staffing rule, a project lead writes issues and decisions. A controller that serves every project belongs to MMDEP.
5. The same tool answers MCRM's need¶
MCRM's open documentation PRs wait for a second agent to review each one and report against Atul's question, so that Atul can approve them. This controller does that if: - the input is a PR (diff plus the files it touches) and a question (from the issue or the PR); - the output is a report comment on the PR.
One audit controller, run by MMDEP, would cover: - PE evidence audits; - MCRM document reviews; - the cross-vendor review that spec §8 asks for.
6. How to resume (proposal, for discussion)¶
Short term (fixes, if PE wants it back soon), about one small story:
1. Take the target from the issue, not from --ref: a Branch: or PR: field in the Work Request. Clone that ref shallowly (--depth 1) and delete the clone after the run.
2. Drop the local checkout from the evidence. Use only its origin URL.
3. Build the evidence from the issue's context pack (STD-AI-CONT-001 §2, with explicit file paths; tier T2 for Flash-lite), plus the PR diff when the target is a PR. Remove the #315 file list and regex.
4. Add to the prompt: "State which files you saw. Claim that code is absent only if the files covering it were supplied; otherwise list it as unknown."
5. Pass stdin=subprocess.DEVNULL to every subprocess. Read the key from Windows Credential Manager (keyring), not an environment variable.
6. Add a footer to each report: Agent: gemini-api/<model> (planner <model>), Role: PE-REVIEW, Seat: SEAT-MAIN, Run: <id>.
7. Record token usage and estimated cost in the run record.
8. Keep the controller in a repository with its tests. Remove the copies.
Proper home (MMDEP), after the discussion:
- Story AC-1, an audit controller in MMDEP. It is the same design, made generic:
- targets: an issue or a PR;
- questions: from the Work Request;
- models: from the agent registry (spec §5B routing by data class);
- one pass per call, so the dispatcher, Task Scheduler or a /loop can run it.
- Forge writes stay narrow: comments and lifecycle labels only, on the configured repos, with a fine-grained token in Credential Manager. A separate bot account would make the author honest instead of "Atul". This changes spec §15 ("MMDEP is read only on forges") and needs an approve item.
- ChatGPT stays PE's lead and only writes Work Requests (and labels them). MMDEP runs the auditors. Atul reads the reports and approves.
- The same controller for MCRM PRs. This is the quickest way to clear the review backlog.
7. Atul's answers (10 Oct) and status of the pilot's own priorities¶
- Key tier: free (P6 is now High).
- AC-1: agreed; it becomes an audit story.
- Bounded context was intended: the controller was meant not to read the whole repository, because of the models' context limits. It succeeded in that; it went too far, because no code reaches the model at all (P3). The fix is the middle path: an explicit file list (context pack) plus the PR diff.
- Verbose mode was for testing, because the timeouts didn't fit different jobs. The cause is one timeout (
--timeout-seconds, default 60) for both stages, whatever the evidence size. AC-1 should set a timeout per stage, scaled to input size, and record each call's duration. - First project: to be chosen against the expectations set in the working-arrangement discussion. GitLab projects need a way for the reviewer chat to read them (open).
The pilot's priorities, checked against v3's code and the one live run:
| Priority | In the code | Proven live |
|---|---|---|
| 1. Configuration and unique run IDs | Yes: run ID, per-run folders, options instead of constants. The #315 file list and regex remain | Yes (#317 run) |
| 2. Label lifecycle with re-fetch | Yes, behind --apply-labels, unit-tested |
No: the live run had labels off |
| 3. Context and two-stage evidence capture | Partly: comments and root AGENTS.md/CLAUDE.md. No path-specific instructions, no decision links followed, no code or diff, one capture stage |
No |
| 4. Verified reporting and recovery | Yes: comment URL required, run-ID reconciliation, unit-tested | Partly: one post verified; recovery not exercised live |
| 5. Model stability | Yes: models kept, fallback recorded | Yes (primary models succeeded) |
8. Questions still open¶
- Gemini on the free tier: (a) move to a paid key for private-repo audits, (b) audit only public or synthetic content with it, or (c) use an engine already on a paid plan for private content (spec §5B)?
- Should Gemini's comments keep appearing as
moneymarche-devwith trailers, or should we create a bot account now? - Which project goes first for AC-1: decided after the working-arrangement discussion.
- PE's
AGENTS.md(PR #319) is still open; it should be merged or revised once the common roles and labels are agreed.