Finish the unimplemented surface - #20
Merged
Merged
Conversation
Nine pages, eleven analytics helpers and one column that the code assigned to
but the schema never had. All of them were 500s that no test covered, because
no test had ever requested the page.
Project.template_used. blueprints/project_templates.py set it on every project
created from a template, but no column existed, so SQLAlchemy kept it as a
transient attribute and dropped it at commit -- the provenance was silently
lost -- while the "my templates" page raised AttributeError filtering on it.
Migration 0003 adds it; upgrade, check and downgrade all verified against a
real database.
Nine templates. admin/{edit_user,company_settings,audit_logs,system_status},
azure/{dashboard,configure}, projects/{my_templates,template_preview} and
reports/project_report. Each renders the exact context its view already
passed; the audit log paginates properly because `logs` is a Pagination and
not a list, and the role select emits enum *names* because the view does
UserRole[value].
system_status was returning a literal dict: database "healthy", "245ms"
average response time, 127 requests per minute, 0.2% error rate. None of it
measured, none of it ever changing. It now measures database round-trip time,
cache backend, broker presence, per-integration credential state and process
metrics -- and omits request rate and error rate rather than inventing them,
pointing at /health/metrics instead.
Eleven analytics helpers. optimize_resource_allocation and
generate_project_insights each called a chain of private helpers, nine of
which did not exist, so both raised AttributeError on their first line of real
work. Implemented against real data: utilisation per resource, over-allocation
in units, excess priced at each resource's own unit cost, moves ranked by
severity; and completion rate, spend against approved budget, throughput
across six periods, DCMA health per project. Deterministic -- the same data
gives the same answer -- and Azure OpenAI is not required for either.
The remaining two helpers were never missing. They were attached to the class
at import time with `AzureAIPredictiveAnalytics._x = _x`, which no AST can
see, so the static check had reported them falsely; the earlier claim that
they had been "dedented out of the class" was wrong. They are ordinary methods
now. Both also returned fixed text -- "Project completion rates are stable",
"Budget adherence is within acceptable range" -- for every company on every
request, whatever the data said. What they return now is measured.
tests/test_all_routes.py walks the URL map and requests all 90 GET routes
signed in, and fails on any server error. It also fails if a route is skipped
for want of a placeholder, so a new converter cannot quietly drop coverage.
83 return 200; none return a server error, down from five.
Both KNOWN_MISSING allowlists are now empty.
test_the_downgrade_reverses_cleanly named its target revision: a bare
`db downgrade` moves back one step, so it only asserted what it meant while
0002 happened to be the head.
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.
Nine pages, eleven analytics helpers and one missing column. All of them were 500s that no test covered, because no test had ever requested the page.
Before and after
Walking all 90 GET routes signed in, against a seeded database:
The missing column
blueprints/project_templates.py:80setproject.template_usedon every project created from a template, but no column existed — SQLAlchemy kept it as a transient attribute and discarded it at commit, so the provenance was silently lost on every template-created project. Line 228 then filtered on it and raisedAttributeError.Migration
0003adds it.upgrade,db check(no drift) anddowngradeall verified against a real database.Nine templates
admin/{edit_user,company_settings,audit_logs,system_status},azure/{dashboard,configure},projects/{my_templates,template_preview},reports/project_report.Each renders the exact context its view already passed. The details that matter: the audit log iterates
logs.itemsand paginates, becauselogsis aPaginationand not a list; the role select emits enum names, because the view doesUserRole[value]; and the three endpoints I first pointed atadmin.*areuser_management.*— there are two admin blueprints,/adminand/management, and these pages belong to the second.All nine verified rendering with real content, 9–46 KB each.
system_status was displaying fiction
It returned a literal dict — database
"healthy","245ms"average response time,127requests per minute,0.2%error rate. Nothing measured, nothing ever changing. An administrator opening it to decide whether the system was in trouble was reading numbers that could not move.It now measures database round-trip time, cache backend, broker presence, per-integration credential state, and process metrics. Request rate and error rate are omitted rather than invented — nothing records them — with a pointer to
/health/metrics.Eleven analytics helpers
optimize_resource_allocationandgenerate_project_insightseach called a chain of helpers, nine of which did not exist, so both raisedAttributeErroron their first line of real work.Implemented against real data. Resource optimisation reports utilisation per resource, names what is over-allocated and by how many units, prices the excess at each resource's own unit cost, and ranks the moves by severity. Portfolio insight reports completion rate, spend against approved budget, throughput across six periods, and DCMA health per project.
Both are deterministic — a test asserts the same data gives the same answer, which is the property a schedule review needs — and neither requires Azure OpenAI.
Where the data cannot support a claim, they say so rather than guessing:
Benchmarking is against the DCMA 14-point standard the platform already implements, not an invented "industry average".
A correction to my earlier report
The other two helpers were never missing. They were attached to the class at import time with
AzureAIPredictiveAnalytics._x = _x, which no AST can see — so the static check reported them falsely, and my claim that they had been "dedented out of the class" was wrong. They are ordinary methods now.Both did return fixed text — "Project completion rates are stable", "Budget adherence is within acceptable range" — for every company on every request, whatever the data said. Tests assert those exact sentences no longer appear.
tests/test_static_integrity.pynow documents that blind spot, so the next finding is treated as a lead to confirm withhasattrrather than a fact.Guarding it
tests/test_all_routes.pywalks the URL map — not a hand-written list — and requests every GET route signed in, failing on any server error. It also fails if a route is skipped for want of a placeholder, so adding a converter cannot quietly drop coverage.Both
KNOWN_MISSINGallowlists are now empty.One test was asserting less than it looked
test_the_downgrade_reverses_cleanlyran a baredb downgrade, which moves back one revision. It only asserted what it meant while0002happened to be the head; adding0003made it pass through an untested state. It names its target revision now.Verification
327 passed, 2 skipped locally (was 315).
ruff format --checkandruff checkclean.🤖 Generated with Claude Code