m12-pr06: P1-11 baseline test triage — 7 stale tests fixed, 4090/4090 green#24
Open
BalaShankar9 wants to merge 1 commit into
Open
m12-pr06: P1-11 baseline test triage — 7 stale tests fixed, 4090/4090 green#24BalaShankar9 wants to merge 1 commit into
BalaShankar9 wants to merge 1 commit into
Conversation
… green
Triaged the entire backend+ai_engine pytest baseline. 7 failures were
stale tests pointing at moved or renamed code (no production bugs).
Each test updated to inspect the current source-of-truth.
Test fixes:
1. backend/tests/aim/test_aim_pipeline_events.py
test_generate_section_emits_writer_reviewer_pair_per_attempt_and_complete_on_pass
- Filter writer/reviewer counts to event_type == "agent_status".
- generate_section also emits an `attempt` event tagged agent=reviewer
carrying the per-attempt payload; that is the third event the old
count caught. Lifecycle counts must match the convention used by the
other tests in this file.
2. backend/tests/unit/test_observability_w3.py
test_metrics_endpoint_exposes_llm_and_cost_gauges
3. backend/tests/unit/test_perf_optimizations.py
test_metrics_endpoint_exposes_cache_and_phase_gauges
4. backend/tests/unit/test_quality_scorer.py
test_metrics_endpoint_exposes_doc_quality_gauges
- Metrics moved out of backend/main.py into the dedicated
backend/app/core/prometheus_collectors.py module. All gauge markers
live there now. Tests updated to inspect the collectors module.
5. backend/tests/unit/test_prod_readiness_audit.py
test_gemini_provider_invokes_circuit_breaker
- The outer _generate_content_throttled is a thin Langfuse-tracing
wrapper. The real SDK chokepoint and breaker gate live in
_generate_content_throttled_inner. Test updated to inspect _inner.
6. backend/tests/unit/test_prod_readiness_audit.py
test_webhook_idempotency_migration_exists
- Migration file lives at supabase/migrations/20260420000000_*.sql
(Supabase 14-digit prefix), not the originally-planned
database/migrations/20260420_*.sql path.
7. backend/tests/security/test_tenancy_isolation.py
test_unauthenticated_resource_routes_return_401
- /api/missions was removed; route returned 404 instead of 401.
Switched to /api/jobs/{job_id} which is a stable resource route
still gated by get_current_user.
Verification:
$ python -m pytest backend/tests ai_engine/tests --no-cov -q --tb=no
4090 passed, 26 skipped, 22 warnings in 259.79s
Blueprint: P1-11 NEEDS-TRIAGE → SHIPPED (m12-pr06).
Net diff: 7 files, ~30 lines.
There was a problem hiding this comment.
Pull request overview
Updates stale test anchors after recent code moves/renames and records the triage as shipped in the architecture blueprint, bringing the baseline back to fully green.
Changes:
- Fix AIM pipeline event test to count only lifecycle (
agent_status) events for writer/reviewer. - Update metrics “source greps” to inspect
backend/app/core/prometheus_collectors.pyinstead ofbackend/main.py, and adjust a circuit-breaker source anchor to target the real Gemini chokepoint. - Update test paths/probes to reflect repo layout and API route removals (Supabase migrations path; swap removed
/api/missions/{id}to/api/jobs/{id}).
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| docs/architecture/WORLD_CLASS_ARCHITECTURE_BLUEPRINT.md | Marks P1-11 as SHIPPED with the final baseline status. |
| backend/tests/unit/test_quality_scorer.py | Points metrics marker assertions at prometheus_collectors source. |
| backend/tests/unit/test_prod_readiness_audit.py | Anchors breaker check to _generate_content_throttled_inner; updates migration path. |
| backend/tests/unit/test_perf_optimizations.py | Points metrics marker assertions at prometheus_collectors source. |
| backend/tests/unit/test_observability_w3.py | Points metrics marker assertions at prometheus_collectors source. |
| backend/tests/security/test_tenancy_isolation.py | Switches unauthenticated probe to stable /api/jobs/{job_id} route. |
| backend/tests/aim/test_aim_pipeline_events.py | Filters event counts to agent_status to avoid counting attempt events. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
P1-11 baseline test triage — 7 failures fixed, 4090/4090 green
Triaged the entire
backend/tests+ai_engine/testsbaseline. All 7 failingtests were stale: they pointed at code that has since moved or been renamed.
No production bugs were uncovered — every fix is in test code only.
Failures and fixes
test_aim_pipeline_events::test_generate_section_emits_writer_reviewer_pair_per_attempt_and_complete_on_passagent=reviewer; pipeline emits 2 lifecycle (agent_status) + 1attemptevent tagged with the same agentevent_type == "agent_status"(matches sibling tests' convention)test_observability_w3::test_metrics_endpoint_exposes_llm_and_cost_gaugesbackend/main.py; metrics movedbackend.app.core.prometheus_collectorstest_perf_optimizations::test_metrics_endpoint_exposes_cache_and_phase_gaugestest_quality_scorer::test_metrics_endpoint_exposes_doc_quality_gaugestest_prod_readiness_audit::test_gemini_provider_invokes_circuit_breaker_inner_generate_content_throttled_innertest_prod_readiness_audit::test_webhook_idempotency_migration_existsdatabase/migrations/20260420_*.sql; actual file issupabase/migrations/20260420000000_*.sqltest_tenancy_isolation::test_unauthenticated_resource_routes_return_401/api/missions/{id}was removed → returns 404 not 401/api/jobs/{job_id}Verification
Blueprint
Section 18:
P1-11NEEDS-TRIAGE→SHIPPED (m12-pr06) — 7 stale tests pointing at moved/renamed code; 4090/4090 green.Net diff
7 files, +26 / -10. Test code only + 1 blueprint line.
Stack
Stacked on m12-pr05 (#23). Burns down item #2 in the m12 ranked plan.
Next up: m12-pr07 (P1-8 cost attribution).