feat(eval): add Native Agent observation product - #1
Conversation
Why: - Eval v0.1 exposed only an artifact scaffold and could not be selected by a Native Agent without an external runtime. - Harness must not own observation execution, and failed observations must remain isolated from the primary task result. What: - Add the evalctl Go CLI with observe, validate, summarize, and compare commands. - Add v2 module-neutral contracts, private crash-atomic storage, revision-chain validation, and deterministic typed analysis. - Add the implicit Eval Skill, personal Plugin metadata, fixtures, and repository verification gates while preserving human product decisions. Validation: - scripts/verify.sh - Go test, race test, vet, formatting, and Darwin/Linux/Windows cross-builds - Fresh Codex explicit invocation and completed/failed/abandoned implicit matrix - Denied-state and missing-Core E2E confirmed no append or task-result mutation - Remaining limitation: Windows Store access fails closed until owner and DACL verification is implemented
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8a6a2716ee
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| plugin_validator="/Users/jgoneit/.codex/skills/.system/plugin-creator/scripts/validate_plugin.py" | ||
| skill_validator="/Users/jgoneit/.codex/skills/.system/skill-creator/scripts/quick_validate.py" |
There was a problem hiding this comment.
On every checkout that does not have the author's exact /Users/jgoneit/... directory layout, scripts/verify.sh reaches the Plugin/Skill validation step and unconditionally fails its -f checks, so the advertised repository verification cannot run in CI or on another developer's machine. Discover these validators from installed tooling or vendor/pin them as repository dependencies instead of embedding one user's home path.
Useful? React with 👍 / 👎.
| if row.Observation == nil { | ||
| rows = append(rows, row) | ||
| continue |
There was a problem hiding this comment.
When a later correction is strict JSON with a valid future recorded_on and task_id but fails schema validation, parsing leaves Observation nil while retaining the task identity hint. This branch therefore keeps the future row for every earlier --as-of, after which ValidateLog invalidates the historical task chain; consequently, appending a bad correction on August 25 can retroactively remove a task from an August 20 report. Retain a trustworthy canonical recorded_on hint and use it to exclude future schema-invalid rows from the snapshot as well.
Useful? React with 👍 / 👎.
| if err := syncDirectory(directory); err != nil { | ||
| return classifyError("sync-directory", directory, err) |
There was a problem hiding this comment.
If directory synchronization fails after atomicReplace succeeds, the new row is already visible at the live path, but this return propagates as an observation failure and --best-effort emits {"status":"skipped"}. An explicit caller can then retry under the false assumption that no append occurred and create a duplicate task observation. The post-replacement outcome needs to distinguish a committed-but-not-confirmed write rather than reporting it as skipped.
Useful? React with 👍 / 👎.
| var value any | ||
| if err := json.Unmarshal(data, &value); err != nil { |
There was a problem hiding this comment.
When a count is written with a sufficiently long fractional tail, this second decode converts it to float64 even though the first decoder used UseNumber; for example, 999999.99999999999999999999999999 becomes the integer-valued float 1000000. Schema validation then accepts it as an integer and the draft is stored with a different value, silently corrupting the observation instead of rejecting the fractional count. Return the exact value already built by strictValue or decode the final value with UseNumber as well.
Useful? React with 👍 / 👎.
| if info.Mode()&os.ModeSymlink != 0 || !info.Mode().IsRegular() { | ||
| return storeError(CategoryUnsafePath, "inspect-file", path, ErrUnsafePath) | ||
| } | ||
| if !privateMetadataOK(info, 0o600) { | ||
| return storeError(CategoryPermission, "inspect-file", path, ErrPermission) |
There was a problem hiding this comment.
When observations.jsonl has another hard link in a source checkout or other non-private location, this check still accepts it because the inode is regular, owner-controlled, and mode 0600. The next atomic replacement only replaces the private pathname, leaving the other link with the prior raw observations available for accidental tracking or retention, violating the external-private-state boundary. Reject data files whose platform link count is greater than one before reading or replacing them.
Useful? React with 👍 / 👎.
| for fixture in "${valid_fixtures[@]}"; do | ||
| "$temp_dir/evalctl" validate --file "$fixture" >/dev/null || | ||
| die "valid fixture rejected: ${fixture#"$repo_root/"}" |
There was a problem hiding this comment.
If either whitelisted valid fixture is accidentally replaced or extended with a real observation, evalctl validate succeeds because population: "real" is schema-valid, and the later repository-boundary check permits the file solely by pathname. The previous verifier explicitly asserted that every valid fixture row was synthetic; removing that guard allows raw real-task data to pass verification and be committed. Inspect both valid fixture sets and reject any row whose population is not synthetic.
Useful? React with 👍 / 👎.
| "read_observation_versions": [ | ||
| "eval-observation/v1", | ||
| "eval-observation/v2" |
There was a problem hiding this comment.
Now that analysis explicitly reads both observation versions, the advertised templates/decision-report-v1.md artifact is stale: its metadata still hardcodes Eval Observation Protocol v1 and eval-observation/v1. A human following that template for a report containing v2 rows will therefore publish incorrect protocol and schema provenance. Version or update the report template so it identifies the current protocol and records all observation schemas present in the dataset.
Useful? React with 👍 / 👎.
|
이 PR은 clean-slate 20-Task experiment로 범위를 다시 정한 PR #2로 대체합니다. prototype 구현은 commit 기존 review 7건의 처리 구분은 다음과 같습니다.
상세 comment별 근거와 검증 결과는 replacement PR 본문의 대응표에 기록했습니다. 이 PR은 merge하지 않고 닫으며, prototype remote branch도 제거합니다. |
1 participant
목적
Eval v0.1은 contract scaffold만 제공해 Native Agent가 직접 관측을 기록·검증·분석할 제품 실행 표면이 없었습니다. 이 PR은 Harness에 runtime 책임을 추가하지 않고 Eval 저장소가 소유하는
evalctl과 terminal Skill을 추가합니다. 관측 실패는 원 Task 결과를 변경하지 않으며, privacy review와 retain/promote/remove/release 판단은 계속 사람이 수행합니다.변경 내용
evalctl에observe,validate,summarize,compare명령과 고정 exit-code 계약을 추가했습니다.eval-observation/v2와 분리된 private state store에만 기록합니다.used:true + version unavailable을 usage 집계에는 포함하고 version comparison에서만 제외하도록 version 상태를 분리했습니다.evalSkill과 Plugin manifest를 추가했습니다.이 PR은 신규 Core, schema, platform별 store 구현, invalid fixtures와 golden outputs를 함께 도입해 변경 파일 수가 큽니다. 서로 독립적으로 배포하면 공개 계약과 verifier가 어긋날 수 있어 하나의 제품 전환 단위로 유지했습니다.
설계 판단
--best-effort로 한 번만 호출합니다.검증 방법
검증 결과
scripts/verify.sh통과go test ./..., race test,go vet ./..., formatting checkgit diff --check통과used:true + version unavailable이 usage에는 포함되고 version comparison에서만 제외되는 회귀 테스트가 통과했습니다.$eval validate가 현재 설치를 유효하게 판정Eval: observation recorded.가 정확히 한 번 표시됨검증하지 못함:
영향 범위
evalctl공개 CLI와 exit-code 계약Harness runtime이나 gitlink/pin은 변경하지 않습니다.
리스크
롤백 방법
8a6a271을 revert하면 v0.1 contract scaffold 상태로 돌아갑니다./Users/jgoneit/.grok/bin/evalctl은 repository 밖의 로컬 상태이므로 필요하면 별도로 제거해야 합니다.리뷰어가 중점적으로 봐야 할 부분
Agent Note
evalctl observe --best-effort만 사용하며 다른 모듈을 version 확인 목적으로 실행하지 않습니다.