Repository navigation
Conversation
Signed-off-by: zengchuang <zengchuang3@huawei.com> Agent-assisted-by: Codex (author)
There was a problem hiding this comment.
LGTM
PR review verdict — PASS
Reviewed base 36440109d4a81ec33fdd2e5ac28449a7fd0ddea9 and head 542d04fd1f440e68445e83779b50c8e983d89a44.
Governing slice: #371 step 6 fixed-status entry only (report / Dashboard / trace / joint CPU acceptance explicitly deferred).
Verified:
read_attempt_viewkeeps execution facts separate from optional archived claims; mount-verified bytes via ArtifactReader / read_assessment; IdentityMismatch on swapped record or assessment revision; ArtifactIntegrityError on broken assessment semantic id.- CLI
status --record-ref+--storage-config(+ optional--assessment-ref) shares the Python projection; mutually exclusive with--run-id; legacystatus --run-id/reportunchanged; PublicError maps to argparse exit 2. - Local
tests/test_apps_assess.py28 passed; ruff clean on touched paths. - Ready CI on head: full-check (3.10/3.11), installed-package (3.10/3.11), declarations, pr-references, dashboard-browser — all success.
No outstanding blockers for this bounded slice. Did not merge.
amy-why-3459
left a comment
There was a problem hiding this comment.
LGTM
PR review verdict — PASS
Reviewed base 36440109d4a81ec33fdd2e5ac28449a7fd0ddea9 and head 542d04fd1f440e68445e83779b50c8e983d89a44. The scope is only the #371 step 6 fixed-status entry; report, Dashboard/live, trace and joint CPU acceptance are deferred, as the body says.
What I checked:
src/inferrsi/apps/views/attempt.py: theRecordRefshape and its JSON media type are checked first. The bytes are read throughArtifactReaderwith the configured mounts, and the record must be a v1 committed record. Every projected execution field,cleanupandcandidate(nullable) is required byExecutionRecordPayloadincontract-v1.schema.json, so a missing field is correctly reported asArtifactIntegrityError. Anattempt_id/record_versionmismatch against the ref raisesIdentityMismatch.- With
--assessment-ref, the archived assessment must match this exactrecord_ref,attempt_id,prepared_ref,protocol_digestandcandidate. It is then rechecked withcheck_assessment_identity. Claim outcomes stay inassessment, separate fromexecution, and both refs are deep-copied, as the mutation assertion in the test shows. - CLI (
arguments.py,cli.pyL159-181):--run-idand--record-refare mutually exclusive. The extra flags are rejected without--record-ref, andPublicError/ValueErrorend inargument_parser.error, i.e. exit 2, matchingdocs/operations.md. Legacystatus --run-idandreportkeep their existing run-directory path. - Tests cover Python/CLI output equality, a swapped record revision, an assessment for another revision, a tampered verdict, the flag combinations, and a fresh-process read of the reassessed SQLite revision.
- All 7 checks on this head are green. No blocking reviews on this head.
Optional, non-blocking: the CLI parses the ref and config files with json.loads. Using inferrsi._json_v1.decode would apply the same v1 JSON rules as the other fixed-ref inputs.
|
Thanks for the decoder check. I kept ordinary JSON formatting for the CLI's reference and trusted configuration files so callers can copy a ref from the formatted CLI output or save an indented configuration. The selected record and assessment bytes still pass through the strict v1 decoder, the pinned size and SHA-256 checks, and the record/assessment identity checks. |
Governing issue and delivery slice
Refs #371, step 6. Builds on the merged Apps views and Harness records migration in #425.
inferrsi.apps.api.read_attempt_viewand CLIstatus --record-ref <record.json> --storage-config <storage.json>use the same read-only projection. An optional--assessment-ref <assessment.json>adds archived outcomes and their scopes for that exact record.status --run-idandreportroutes use their existing run-directory reader.Evidence
222 tests and 2 subtests passed on Windows across Apps, CLI, module boundaries and frozen legacy runs. The tests compare CLI and Python output, reject swapped revisions and invalid archived assessment identities, and read an older SQLite record with its matching archived assessment from a fresh process. Repository Ruff and staged diff checks passed.