PlantAI-dev/PLAN-REVIEW-LOG.md

Plan Review Log: PPI stage-local STRING ID resolution

Act 1 discussion complete — plan locked with the user. MAX_ROUNDS=5.

Phase 0 evidence consolidation

Local code inventory

• skills/ppi/run.py currently strips versions in both Ortholog and direct paths, has no get_string_ids, writes probe_status=not_run, and conflates network API errors with [].
• apps/web/pipeline_backend.py creates pep_cds_idmap.tsv, but stores LOC... in gene_id; numeric NCBI GeneID exists only inside cds_header.
• PPI main and PPI Reset are separate backend entry points and both must pass the same ID evidence.
• Existing AlphaFold link generation already prefers string_id, so a populated resolution report can be reused.
• Existing frontend can preview the TSV; direct metric labels are an enhancement required by the approved acceptance criteria.

Official STRING API evidence

• Official docs: https://string-db.org/help/api/ and fixed v12 docs: https://version-12-0.string-db.org/help/api/.
• get_string_ids returns one best STRING match per input; limit is not a supported parameter.
• Use queryIndex rather than response order; unresolved inputs are omitted.
• All-unresolved get_string_ids has been observed/documented as HTTP 200 with [] or HTTP 404; network can also return 404 for unusable inputs. Endpoint, response shape, and request state—not status alone—must drive classification.
• network should explicitly set add_nodes=0 for input-only family networks.
• Validate score, network type, taxon, and non-empty identifiers client-side.
• Version response currently uses stable_address; client should also tolerate documented string_stable_address.
• Official guidance recommends POST for lists, caller_identity, stable-version URLs, and at least one second between requests.

Round 1 — Codex

The biological direction is sound—canonical IDs remain immutable, Ortholog remains a relationship, GeneID is a valid job-local bridge, partial mapping is distinguishable from an empty network, and the scope excludes unrelated Ortholog/KaKs redesign. However, the plan is not implementation-ready.

Blocking changes:

1. Close the PPI Reset identity and scope path.
   • Reset currently reloads Ft canonical IDs but does not carry the Ft→soe Ortholog relationship; the soe alias map alone is insufficient.
   • Carry the exact scoped Ortholog file and verified ID map into Reset, select the exact scoped source manifest, constrain client-supplied paths, declare Reset input namespace, and test real Ft→soe Reset wiring.
2. Make provenance cardinality explicit before deduplication.
   • The real job has 24 Ft canonical members, 12 unique soe reference queries, and many-to-one relationships. Preserve every canonical→reference path and deduplicate only final STRING network queries.
   • Cover conflicting references, versionless collisions, conflicting GeneIDs, and mixed-species ID-map rows.
3. Define metric denominators and exact manifest locations.
   • Distinguish canonical members, Ortholog relationships, unique reference queries, resolved STRING queries, returned network nodes, and edges.
   • Main, scoped, and Reset manifests must expose the same scientific metrics; frontend tests must assert values, not label presence only.
4. Reconcile partial mapping with evaluate_string_mapping_quality().
   • Partial nonzero resolution must be nonterminal; zero resolution must fail; ambiguous entries must be excluded.
5. Lock the STRING HTTP state machine.
   • Handle all-unresolved 404 and 200 [], validate queryIndex and taxon, test throttling, always send add_nodes=0, and distinguish stable scientific endpoints from optional visualization endpoints.
6. Specify header-only empty-network artifacts and persisted failure manifests, including CLI exit codes.
7. Expand validation to scoped Reset, exact manifest selection, cross-job/species rejection, Reset APIs, stage status, 24→12 provenance, and manifest equivalence.

VERDICT: REVISE

Codex's response

Accepted all seven blockers because each is supported by current code or real-job evidence and remains inside the approved PPI-first boundary. The revised plan will:

• make Reset canonical-target-only, scope-bound, and dependent on both the exact Ortholog relationship and species-matched alias map;
• introduce an explicit per-canonical relationship record and retain 24 provenance rows before 12-query deduplication;
• replace ambiguous metrics with nested, denominator-defined ID-resolution and network metrics shared by main/scoped/Reset manifests;
• retire the old terminal low-rate threshold in favor of complete/partial/failed resolution states;
• define the STRING response validator, request pacing, empty-network TSV schema, failure manifests, exit codes, and expanded API/real-data validation.

The optional STRING image/link endpoint will remain explicitly non-authoritative visualization evidence if it cannot use the same stable version endpoint; it will not determine scientific stage status.

Round 2 — Codex

The revision closes the original seven blocker groups in substance. Four material issues remain:

1. query_mapping_rate uses the wrong numerator when multiple reference queries converge on one STRING entity. Add resolved_reference_query_count; keep selected_string_query_count as the deduplicated network-query count and test two references converging on one STRING ID.
2. Reset needs race-safe evidence snapshots and a separate direct-target policy. Snapshot and re-hash canonical input, Ortholog mapping, and ID map inside the isolated Reset directory before launch; validate direct-target IDs against the recorded canonical member set.
3. Cross-process STRING pacing is not yet testably specified. PPI uses a light queue with concurrency four, so choose a shared interprocess gate or a dedicated single-concurrency queue and test it.
4. Contract/failure validation is incomplete. Explicitly test persisted failed manifests and backend/status exposure for CLI exits 2/3/4; synchronize skills/ppi/config.yaml; decide whether network_type is fixed or configurable; replace the stale flat metric schema in the project record.

VERDICT: REVISE

Codex's response

Accepted all four findings.

• Add resolved_reference_query_count and use it for query mapping rate/status; network-query deduplication remains a separate count.
• Record the canonical input hash and make Reset execute from verified snapshots copied into its isolated directory. Projected Reset validates against Ortholog sources; direct-target Reset validates against the recorded canonical member set.
• Use a host-shared fcntl.flock + timestamp file gate for all stable STRING version/mapping/network requests across main and Reset processes; production interval remains one second and a short-interval multiprocessing test proves serialization.
• Add backend/status failure-manifest tests, synchronize skills/ppi/config.yaml, and keep this phase's PPI analysis fixed to network_type=functional to avoid unrelated request/UI expansion. Update the original project-record metric section to the nested schema.

Round 3 — Codex

Round 2 is almost fully closed. Two material blockers remain:

1. The shared STRING rate gate excludes automatic image requests; route every fallback attempt through the same interprocess gate or disable retrieval, and test a scientific request concurrent with an image request.
2. The direct-target Reset rule conflicts with a following blanket rule requiring all added IDs to be Ortholog sources; qualify that rule as projected-only.

VERDICT: REVISE

Codex's response

Accepted both findings. The detailed plan now places the host-shared gate in gfa_core.string_rate_gate and requires every active PPI-path STRING HTTP call—including each optional image/SVG fallback—to use it with the same caller identity. The multiprocessing suite combines scientific and image callers. The Ortholog-source requirement is explicitly projected-Reset-only; direct-target Reset accepts only IDs present in the recorded canonical snapshot.

Round 4 — Codex

All prior blockers are closed.

• gfa_core.string_rate_gate is the single host-shared gate for version, mapping, network, and every image/SVG fallback in main and Reset, with aligned caller identity and mixed scientific-plus-image multiprocessing coverage.
• Projected Reset additions must be Ortholog sources; direct-target Reset accepts only IDs from its recorded canonical snapshot. The rules and tests no longer conflict.
• The earlier metric, snapshot, failure-manifest, configuration, network-type, and validation findings remain closed.

Non-blocking caution: the fcntl gate is intentionally host-local; a future multi-host worker deployment would require a distributed gate.

VERDICT: APPROVED