QT/sure
1
0
Fork 0
mirror of https://github.com/we-promise/sure.git synced 2026-08-05 16:42:18 +00:00
Code Issues Packages Projects Releases Wiki Activity

Correct the vault tools' reconciliation claims and citation parsing

Browse Source 
Review findings from @diegomarino, all verified against the code before
changing anything.

The reconciliation claim was the serious one. get_account_statement told
agents the checks were "the trustworthy part" and returned "the balances read
off it" — but nothing reads balances off a document. MetadataDetector never
touches them and create_from_prepared_upload! never sets them; they are
user-editable fields in the Statement Vault UI. So a statement archived over
MCP always came back with an empty check list, which an agent could easily
read as "the document agrees with the ledger" when it means "nobody has
entered the figures". The description now says so, and the payload carries a
reconciliation_note spelling it out for anything reading only the JSON. Also
noted that these checks are ledger agreement, not parse integrity: nothing
here verifies a document's parts sum to its printed total.

Provenance::Citation had two patterns disagreeing about spacing. GRADE_SUFFIX
allowed "(grade:A)" but FORMAT required exactly one space, so that citation
passed the pre-check and then parsed as ungraded with the grade swallowed into
the text — silently discarding the reliability the caller supplied, which is
the one thing this parser exists to prevent.

list_account_statements downcases content_sha256 before querying. The column
is constrained to lowercase hex, so uppercase input could never match, and an
agent would read the empty result as "not archived" and upload a duplicate.
Its period filters are renamed overlapping_from / overlapping_until, since
they match on overlap and the old names claimed otherwise to anyone reading
the schema without the descriptions. has_more now explains that there is no
cursor and the way forward is a bigger limit or narrower filters.

record_valuation no longer overwrites the entry's notes. Re-recording a date
would destroy a note a person had written there. Nothing is removed now: an
identical citation is a no-op, a changed one is appended, and the trail of
what was cited when survives. Detecting "did this tool write that line?" is
not possible — almost any prose parses as a valid ungraded citation — so the
code does not guess.

Minor: accept urlsafe base64 on upload, and explain in the code why
record_valuation checks the account ACL rather than the vault manager role, so
nobody "tightens" it into the wrong permission later.

Tests cover each: the grade-spacing cases both ways, uppercase SHA lookup,
overlap window boundaries, note preservation and no-stacking, the unavailable
reconciliation note appearing and disappearing, and — per the review — that
the download URL's signed id actually expires, rather than trusting the
description's claim.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JFDp9HhXDeswadu4cxFojn
This commit is contained in:
Claude
2026-08-01 17:15:24 +00:00
parent 9ebc5e8daa
commit 74b3fbac8b
10 changed files with 237 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files

44
app/models/assistant/function/get_account_statement.rb
View File

@@ -13,18 +13,29 @@ class Assistant::Function::GetAccountStatement < Assistant::Function
def description
<<~INSTRUCTIONS
Fetch one Statement Vault document by ID: its identity (SHA-256, filename,
period), the balances read off the document, and the reconciliation checks
comparing those balances against the ledger.
period), any balances recorded for it, and reconciliation checks against
the ledger.
The reconciliation checks are the trustworthy part. Each compares a figure
printed on the statement against the account's recorded balance:
IMPORTANT — reconciliation is usually empty, and its absence means nothing.
The checks only exist once a human has typed the statement's opening and
closing balances into the Statement Vault UI. Nothing extracts them from
the document, so a statement you uploaded through `upload_account_statement`
comes back with `reconciliation_checks: []` and
`reconciliation_status: "unavailable"`. That is "nobody has entered the
figures", NOT "the document agrees with the ledger". Never report an
unreconciled statement as verified.
When the balances have been entered, each check compares that figure
against the account's recorded balance:
- `opening_balance` / `closing_balance` — the statement's figure vs the ledger's
- `period_movement` — the change across the period, both sides
Each check reports `matched` or `mismatched` (tolerance: 0.01). A mismatch
means the ledger and the document disagree — report it, and do not paper
over it by adjusting the number to fit.
Each reports `matched` or `mismatched` (tolerance: 0.01). A mismatch means
the ledger and the document disagree — report it, and do not paper over it
by adjusting the number to fit. Note this is ledger agreement only: nothing
here verifies that the document's own line items sum to its printed total.
That parse-integrity check belongs to whatever extracted the figures.
Also returns a short-lived download URL (valid #{DOWNLOAD_URL_TTL.inspect})
for the original file when one is attached.
@@ -63,17 +74,32 @@ class Assistant::Function::GetAccountStatement < Assistant::Function
return error("not_found", "No statement found with that ID that this user can view.")
end
checks = reconciliation_payload(statement)
status = statement.reconciliation_status
{
success: true,
statement: statement_payload(statement).merge(
reconciliation_status: statement.reconciliation_status,
reconciliation_checks: reconciliation_payload(statement),
reconciliation_status: status,
reconciliation_checks: checks,
# Spelled out in the payload, not just the tool description: an agent
# reading only the JSON must not read an empty check list as agreement.
reconciliation_note: unavailable_note(status),
download_url: download_url(statement)
).compact
}
end
private
def unavailable_note(status)
return nil unless status == "unavailable"
"No reconciliation has been performed: this statement has no opening/closing " \
"balances recorded, and nothing extracts them from the document. Someone must " \
"enter them in the Statement Vault UI. This is not evidence that the statement " \
"agrees with the ledger."
end
def reconciliation_payload(statement)
statement.reconciliation_checks.map do |check|
{

36
app/models/assistant/function/list_account_statements.rb
View File

@@ -27,12 +27,16 @@ class Assistant::Function::ListAccountStatements < Assistant::Function
uploaded documents use `search_family_files`; to fetch one statement's
reconciliation figures and a download link use `get_account_statement`.
There is no cursor or offset. `has_more: true` means the result was
truncated — raise `limit` (up to #{MAX_LIMIT}) or narrow the filters to see
the rest; paging forward is not possible.
Example:
```
list_account_statements({
review_status: "unmatched",
period_start_on_or_after: "2026-01-01"
overlapping_from: "2026-01-01"
})
```
INSTRUCTIONS
@@ -57,15 +61,15 @@ class Assistant::Function::ListAccountStatements < Assistant::Function
},
content_sha256: {
type: "string",
description: "Look up a specific document by the SHA-256 of its contents. Use this to check whether a file is already archived."
description: "Look up a specific document by the SHA-256 of its contents (hex; case-insensitive). Use this to check whether a file is already archived."
},
period_start_on_or_after: {
overlapping_from: {
type: "string",
description: "ISO 8601 date (YYYY-MM-DD). Only statements whose period ends on or after this date."
description: "ISO 8601 date (YYYY-MM-DD). Only statements whose period overlaps this date or later, i.e. whose period ENDS on or after it."
},
period_end_on_or_before: {
overlapping_until: {
type: "string",
description: "ISO 8601 date (YYYY-MM-DD). Only statements whose period starts on or before this date."
description: "ISO 8601 date (YYYY-MM-DD). Only statements whose period overlaps this date or earlier, i.e. whose period STARTS on or before it."
},
limit: {
type: "integer",
@@ -95,18 +99,24 @@ class Assistant::Function::ListAccountStatements < Assistant::Function
scope = scope.where(review_status: status)
end
scope = scope.where(content_sha256: params["content_sha256"].to_s.strip) if params["content_sha256"].present?
# Downcased because the column is constrained to lowercase hex
# (chk_account_statements_content_sha256), so uppercase input would not merely
# be unlikely to match — it could never match, and the agent would read the
# empty result as "not archived" and upload a duplicate.
if params["content_sha256"].present?
scope = scope.where(content_sha256: params["content_sha256"].to_s.strip.downcase)
end
if params["period_start_on_or_after"].present?
date = parse_date(params["period_start_on_or_after"])
return error("invalid_date", "period_start_on_or_after must be an ISO 8601 date (YYYY-MM-DD).") unless date
if params["overlapping_from"].present?
date = parse_date(params["overlapping_from"])
return error("invalid_date", "overlapping_from must be an ISO 8601 date (YYYY-MM-DD).") unless date
scope = scope.where("period_end_on >= ?", date)
end
if params["period_end_on_or_before"].present?
date = parse_date(params["period_end_on_or_before"])
return error("invalid_date", "period_end_on_or_before must be an ISO 8601 date (YYYY-MM-DD).") unless date
if params["overlapping_until"].present?
date = parse_date(params["overlapping_until"])
return error("invalid_date", "overlapping_until must be an ISO 8601 date (YYYY-MM-DD).") unless date
scope = scope.where("period_start_on <= ?", date)
end

30
app/models/assistant/function/record_valuation.rb
View File

@@ -34,7 +34,9 @@ class Assistant::Function::RecordValuation < Assistant::Function
this function — ask the user for the source. A value with no provenance
is worse than a missing value, because it looks authoritative.
Recording a valuation on a date that already has one replaces it.
Recording a valuation on a date that already has one replaces it. The
citation is stored in the entry's notes; any note a person wrote there is
preserved and the new citation appended below it.
Example:
@@ -90,6 +92,12 @@ class Assistant::Function::RecordValuation < Assistant::Function
account_id = params["account_id"].to_s
return error("invalid_account_id", "account_id must be a UUID.") unless valid_uuid?(account_id)
# Unlike the Statement Vault tools, this one deliberately does NOT check
# AccountStatement.statement_manager?. That role governs the document archive;
# writing a value to an account is governed by the account ACL, and
# writable_by is the same scope the human-facing api/v1/valuations endpoint
# uses. Do not "tighten" this by adding the vault role — the two permissions
# answer different questions.
account = family.accounts.writable_by(user).find_by(id: account_id)
return error("account_not_found", "No account found with that ID that this user can write to.") unless account
@@ -109,7 +117,7 @@ class Assistant::Function::RecordValuation < Assistant::Function
end
entry = account.entries.valuations.find_by!(date: date)
entry.update!(notes: citation.to_s)
entry.update!(notes: merged_notes(entry.notes, citation))
end
unless entry
@@ -137,6 +145,24 @@ class Assistant::Function::RecordValuation < Assistant::Function
end
private
# Re-recording a date replaces the valuation, and the citation lives in the
# entry's notes — where a human may also have written something. Nothing is
# ever removed: the note is kept and the citation appended, because we cannot
# tell a tool-written line from a human one (almost any prose is a valid
# ungraded citation) and guessing wrong would destroy the only copy.
#
# Re-recording with the same citation is a no-op, so the common case does not
# grow the note. A genuinely different citation is appended, which is the
# right outcome for a provenance trail: it records that the cited basis for
# this date changed.
def merged_notes(existing, citation)
existing = existing.to_s.strip
return citation.to_s if existing.blank?
return existing if existing.lines.any? { |line| line.strip == citation.to_s }
[ existing, citation.to_s ].join("\n\n")
end
def parse_date(value)
return nil if value.blank?

14
app/models/assistant/function/upload_account_statement.rb
View File

@@ -138,14 +138,18 @@ class Assistant::Function::UploadAccountStatement < Assistant::Function
end
end
# Whitespace is stripped first because agents routinely wrap long base64
# across lines, but decoding stays strict after that: Base64.decode64 quietly
# discards characters it doesn't understand, which would archive corrupted
# bytes under a hash that looks perfectly legitimate.
# Whitespace is stripped because agents routinely wrap long base64 across
# lines, and the urlsafe alphabet is translated to the standard one because
# they sometimes emit it. Decoding stays strict after that: Base64.decode64
# quietly discards characters it doesn't understand, which would archive
# corrupted bytes under a hash that looks perfectly legitimate.
def decode_content(value)
return nil if value.blank?
Base64.strict_decode64(value.to_s.gsub(/\s+/, ""))
normalized = value.to_s.gsub(/\s+/, "").tr("-_", "+/")
normalized += "=" * ((4 - normalized.length % 4) % 4)
Base64.strict_decode64(normalized)
rescue ArgumentError
nil
end