PR State:
merged
PR:
#95
Commit:
a1aff33784b1937129db1037483b720654ed7b7f
[FIX] viin_ai: close independent review findings on #93 cost governance
An independent review of the #93 BYOK cost-cap governance PR approved
it (81/100, zero CRITICAL/HIGH) but flagged 4 MED + 3 LOW to close
before merge. This closes all of them except the review's suggested
MED-1 migration script, which is deliberately NOT added (see below).
MED-2: viin_ai_rag source.py _process_pending caught AICostCapExceeded
in the generic except-Exception branch, landing a capped Firecrawl
source on permanent status='error' - inconsistent with the sibling
media.py fix in this same PR (status='pending', retryable). Adds a
dedicated except clause. viin.ai.source has no 'pending' status value
(unlike viin.ai.media) - 'draft' is the model's actual retryable state
(the search domain that feeds this sweep already re-selects
draft/processing), so the fix mirrors the INTENT (retryable, not
stuck) rather than copying the literal status string, which would
have raised on write.
MED-3: the Firecrawl cap CHECK (_enforce_daily_cost_caps, reads
self.env.company) and the usage BOOKING (_record_firecrawl_usage,
keys on source.company_id) read two different ledgers in a
multi-company DB. _process_pending now scopes each source's whole
processing pass with_company(source.company_id or self.env.company),
so both calls agree on one ledger.
MED-4: adds RED-before-green regression tests for the 3 previously
untested cap-breach callers from the prior commit - brain_tools.py
(re-raise, not silent ILIKE degrade), ir_model_fields.py cron (stop
the whole sweep, not a per-record swallow), media.py (retryable
'pending', not permanent 'error'). Each docstring states the RED
rationale (reasoned statically, no instance run).
LOW-2: documents the budget_cost_usd=0 inversion (0 = defer to system
cap, the OPPOSITE of the ICP caps' 0 = block) in the field help text.
LOW-3: agent.run() can return status='budget_exceeded' with empty
content WITHOUT raising (the per-run Guard 3 path, distinct from the
daily-cap AICostCapExceeded raise already handled) - the discuss bot
silently posted nothing in that case. Now posts the same budget notice
for either signal.
MED-1 NOT applied as suggested: the review proposed a migration script
for the ir_cron.xml noupdate user_id pins. This repo has no deployed
database (AGENTS.md 0.1, no-migration policy) - every install is
fresh, and a noupdate record is created WITH the pin on fresh install,
so the pin already reaches every install. Added a one-line comment
instead; no migrations/ file, no version bump.
Also reconciles docs/ai/architecture.md and docs/ai/security.md
cost-cap sections, which previously flagged #93 as an open/unmerged
defect, to describe the now-fixed behavior (all 3 guards live, stored-0
rejected, Firecrawl metered, breaches surfaced not silently degraded).
Signed-off-by: David Tran <davidtran.hp@gmail.com>
Branch:
17.0