feat(research): add steward golden-set evaluation - #10
seonghobae wants to merge 51 commits into
Conversation
Signed-off-by: Seongho Bae <me@seonghobae.me>
|
@codex review |
|
@coderabbitai review |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
![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: e7d3243fed
ℹ️ 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".
Signed-off-by: Seongho Bae <me@seonghobae.me>
Signed-off-by: Seongho Bae <me@seonghobae.me>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 399794e5ae
ℹ️ 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".
Signed-off-by: Seongho Bae <me@seonghobae.me>
|
@codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Signed-off-by: Seongho Bae <me@seonghobae.me>
|
@codex review |
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
P1 semantic-reconciliation finding on exact fdf8b8d70c05bcb76c55cb6336c9bf31b5e42ce4 against current parent #9 a67d9d66b35024d6f2155f50ee5c9fb7d2e1dbe9.
The stale-parent finding is not only a textual conflict. Current #9 intentionally made ClassificationReport provenance/snapshot/inventory state private and constructor-bound with read-only accessors. This #10 branch's golden-set integrity tests were authored against the older public aggregate and directly read or mutate trusted report fields (library_version, rule_revision, classified_items, unclassified_items, pending_source_item_keys, snapshot_items, observed_item_count). Reopening those fields during restack would regress the exact aggregate-integrity invariant that current #9 now owns. Negative corruption tests must instead use an internal/test-only construction seam or an explicitly untrusted wire/fixture type; golden evaluation production code should consume read-only accessors.
A second compatibility seam is ZoteroItem: #10 adds a mandatory public source_record: Option<serde_json::Value> field and updates its own struct literals to fabricate None. Current #9's public item shape has only key/version/data and existing callers/tests construct that shape directly. A bare merge therefore creates a source-breaking API change and pushes raw-provider provenance responsibility onto callers. Preserve the current public source contract or introduce a backward-compatible constructor/default/separate provider-record boundary; do not require unrelated callers to mint a fake raw record.
Required RED→GREEN for the ordinary non-force reconciliation:
- keep current #9's compile-fail/private trusted aggregate invariant;
- make golden-set evaluation work through read-only report accessors and an explicit internal/untrusted-fixture path for corruption tests;
- preserve raw provider snapshot binding without making
source_record=Nonea caller-owned authenticity signal; - add regressions proving the golden proposal/approval digest changes for every material current-#9 proposal/evidence field while remaining order-stable;
- preserve all #10 golden-label/approval/digest functionality and all current #9 lifecycle, abstention, source-resolution, provenance and publication deltas;
- regenerate exact-head Rust 1.98/fmt/strict Clippy/rustdoc/release/owned coverage and hosted/review evidence after the repaired merge.
The current #10 head has no hosted workflow generation, so no exact-current hosted GREEN exists to carry. Keep Draft/open and repair forward; whole-tree ours/theirs, force push, destructive rebase, public-field rollback, or closing the valid delta is not acceptable.
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Current-head reconciliation finding: #10 is still rooted at historical #9 51c7df6d03f072449422fd58ca24b2f9d6026f07, while canonical #9 is a67d9d66b35024d6f2155f50ee5c9fb7d2e1dbe9; compare remains 35 ahead / 81 behind and non-mergeable. The valid golden-set delta must be adopted by ordinary two-parent integration, but not by reopening #9's private constructor-bound ClassificationReport and not by adding mandatory caller-owned ZoteroItem.source_record. I am treating those as repair findings: preserve #9's public ZoteroItem { key, version, data } construction contract and private report aggregate, move raw-provider capture to an explicit captured/wire boundary, keep golden approval/evaluation aggregate-only, and bind proposal digests to all current #9 lifecycle/evidence fields through read-only accessors. Existing #10 corruption tests that mutate trusted report internals must move to an explicitly untrusted/test-only boundary rather than weakening encapsulation. No force push, destructive rebase, whole-tree ours/theirs, or partial delta discard.
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Exact-head checkpoint after ordinary parent reconciliation and documentation currentization. Current #10 is now based on #9 a67d9d66b35024d6f2155f50ee5c9fb7d2e1dbe9, mechanically mergeable, with the current #9 lib.rs blob preserved and golden evaluation layered through CapturedZoteroItem/GoldenSnapshot rather than reopening trusted fields. Both historical P1 threads (complete raw provider binding and proposal/approval binding) are repaired forward and resolved with exact source coordinates. This is not approval: no repository-owned PR workflow generation exists on 865e3d569..., combined status is CodeRabbit success only, and the current execution host has no cargo/rustc/rustfmt/rustup. Keep Draft until unchanged-head Rust 1.98 + hosted/review acceptance is established.
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Exact-head semantic review at 865e3d569c814fdd3b18d6f4402d2ec9bfd2f3ef found one provenance-contract P1.
CapturedZoteroItem::try_from(Value) is a public caller-constructible boundary. Any caller can fabricate an arbitrary JSON Value, decode it successfully, and pass it to classify_captured_golden_snapshot. The resulting receipt is nevertheless domain-separated as conceptweave-zotero-provider-snapshot-v3, while docs/doctoring/golden_set_reconciliation.md says the typed-fixture domain exists so a fixture cannot be presented as provider-authenticated evidence. Domain separation distinguishes two code paths, but it does not authenticate the origin of bytes supplied through the public constructor. A fabricated caller value can therefore mint the same provider-labelled receipt class as bytes actually obtained from Zotero.
Repair this without inventing provider trust: first add an executable regression fixing the receipt semantics, then minimally rename/reframe the public raw-JSON receipt domain as caller/capture-bound evidence (for example conceptweave-zotero-captured-json-snapshot-v3) and explicitly state that it binds complete bytes/shape but does not prove provider origin. If later genuine provider authentication is needed, it must enter through a transport-owned capture/attestation boundary rather than a caller-selected digest domain. Keep the typed-fixture domain distinct. Because #10 is Draft/unreleased and exact-head acceptance is still absent, do not migrate or backfill old receipts; invalidate/regenerate them under fresh approval.
This is a COMMENT finding, not approval or merge authorization.
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Exact-head follow-up at 6aa1d39512cbbf9d37df518f4109d0e69eef0689.
The provenance finding from review 5192398634 is source-repaired in this lineage. RED 67b5214ce8cd39fddbc1ba6043971b9832b2ef17 fixes the intended contract: a public caller-constructed raw JSON capture must produce the non-authenticating conceptweave-zotero-captured-json-snapshot-v3 receipt class. Production 2e52972745953fbb5b606226fce2dda2cf61abd3 changes the digest domain and rustdoc so raw bytes plus decoded typed input are content-bound but do not claim provider origin. Doctoring, product baseline, ADR, TRD, CHANGELOG and UML were then brought forward through exact 6aa1d395...; the UML's stale v2 proposal-binding reference is also corrected to v3.
Mechanical compare from the RED to current head is ordinary-forward and does not reopen current #9's private ClassificationReport or alter its stable ZoteroItem { key, version, data } contract. The superseded provider-labelled receipt semantics are Draft/unreleased evidence and are invalidated rather than locally migrated.
No exact-head GREEN is claimed: this host has no Rust toolchain, this exact head has zero repository-owned PR workflow runs and no combined commit statuses, and no qualifying independent approval exists. Keep Draft and acceptance-pending.
This COMMENT is not approval or merge authorization.
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Current-head finding: the ordinary reconciliation changed [lib] to src/merged_lib.rs, which then includes the canonical Research Intake src/lib.rs as a nested module. That source file still carries #![cfg_attr(coverage_nightly, feature(coverage_attribute))]. Rust enables unstable compiler features through crate-level #![feature(...)]; nesting that source turns a crate-level capability declaration into a module-level inner attribute when coverage_nightly is active. The normal stable lane can hide this because cfg_attr removes the attribute when the cfg is false, while the intended nightly/coverage configuration becomes structurally invalid. This is also an unnecessary second crate-root seam around the canonical Research Intake owner.
Repair contract: first add a regression that requires the package library root to remain src/lib.rs and requires that root to export the golden-set module. Then minimally attach golden_set from the canonical crate root, restore Cargo's normal src/lib.rs root, and remove the wrapper. Do not reopen ClassificationReport, change the public ZoteroItem shape, weaken coverage, or transfer predecessor GREEN. Exact-head Rust/hosted evidence remains required after the repair.
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Exact-head source checkpoint at bf6ad9a1c613bd4a36a237d914030997d5721b02.
The crate-root finding from review 5192569298 is repaired forward. RED 18e99a5982c326cf9460db590341bf58b735510a pins the canonical-root contract. Production eccfc978fc28f883cfe548eb14e8eab715e3e4bf restores src/lib.rs as the package root, attaches/re-exports golden_set there, removes the Cargo [lib] override, and deletes the temporary wrapper. Mechanical RED→repair compare is one commit ahead / zero behind and limited to those three paths. Documentation successors make the same ownership boundary current.
This is source repair, not GREEN or approval. No predecessor Rust evidence transfers. This exact head still needs repository-pinned Rust 1.98 workspace/fmt/strict Clippy/rustdoc/release/owned coverage plus hosted security/review acceptance and qualifying independent review before Ready or merge.
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Fresh prerequisite checkpoint only; Research source is unchanged.
ConceptWeave protected/default main remains f4f440dd58c77d7cd90dff8a1eb2eeb9a9940425, so Product bootstrap #35 is still required before this exact Research head can obtain repository-owned Product evidence. Central .github/main has advanced to protected ebc69a4016f7668beaef5e3b592d378f22ada684. Review-repair #2170 is now non-force restacked at c741b608322208b8bc222792ceb3b6c63207e157; Noema #2079 is restacked at e7c5044c4a6228850829660e32b1bed342cc5cb3; earlier checks/reviews on their predecessor heads are historical. CodeQL handler #2106 still requires semantic restack because the protected advance overlaps its governed CHANGELOG/gap-baseline edits.
No bf6ad9a1... Research file changed, no predecessor GREEN is transferred, and #11 remains untouched until this parent reaches unchanged-head native/hosted/review acceptance. This COMMENT supersedes only the stale central-coordinate paragraph in the body.
1 participant
Current authority — 2026-09-14 KST
Exact head
bf6ad9a1c613bd4a36a237d914030997d5721b02; base/current canonical Research Intake #9a67d9d66b35024d6f2155f50ee5c9fb7d2e1dbe9. OPEN / Draft / mechanically mergeable. State: STALE_PARENT_REPAIRED / AGGREGATE_COMPATIBILITY_REPAIRED / CAPTURE_PROVENANCE_REPAIRED / CANONICAL_CRATE_ROOT_REPAIRED / ACCEPTANCE_PENDING.The owned delta remains steward-reviewed golden-set evaluation above Research Intake: complete raw/typed snapshot content binding, proposal/report digest binding, source-scope admission, inventory/approval integrity, pending-source ancestry, deterministic classification evaluation and aggregate-only quality evidence. No classifier result, local Zotero metadata, caller-supplied raw JSON, approval JSON or evaluation score becomes governed semantic authority by itself.
Ordinary stale-parent reconciliation
Historical #10
fdf8b8d70c05bcb76c55cb6336c9bf31b5e42ce4was rooted at historical #951c7df6d03f072449422fd58ca24b2f9d6026f07; before repair it was 35 commits ahead / 81 behind current #9 and non-mergeable. Review5192181682required preserving current #9's private constructor-boundClassificationReportand stable publicZoteroItem { key, version, data }instead of reopening trusted fields or adding mandatory caller-ownedsource_recordprovenance.Ordinary two-parent integration
c9954e286041c80d08655e092f10007a684ceab1has parents historical #10fdf8b8d70...and current #9a67d9d66.... Current compare from #9 remains ahead with zero commits behind. No force-push, destructive rebase, whole-tree ours/theirs selection or predecessor-check transfer was used.Current #9's trusted report fields stay private and public
ZoteroItemremains key/version/data only.GoldenSnapshotcomposes evaluation evidence over read-only Research Intake accessors rather than reminting a competing trusted aggregate.Raw-capture provenance repair
Exact-head review
5192398634found that the first reconciliation overstated provenance.CapturedZoteroItem::try_from(Value)is public and caller-constructible: arbitrary fabricated JSON can be decoded and passed toclassify_captured_golden_snapshot. Domain separation can bind content and distinguish code paths, but cannot prove that caller-supplied bytes originated from Zotero. The previousconceptweave-zotero-provider-snapshot-v3label therefore could be misread as provider-authenticated evidence.The repair is ordinary-forward and remains inside #10's Draft/unreleased contract:
67b5214ce8cd39fddbc1ba6043971b9832b2ef17requires caller-constructed raw capture to useconceptweave-zotero-captured-json-snapshot-v3.2e52972745953fbb5b606226fce2dda2cf61abd3changes the digest domain and rustdoc: raw JSON plus decoded typed input are content-bound, while provider origin requires a separate transport-owned attestation boundary.c0a139d82649d44b7fc8a115357ddc7b961315edinvalidates the superseded provider-labelled semantics rather than locally migrating receipts.The typed fixture domain remains
conceptweave-zotero-typed-snapshot-v3; proposal identity remainsconceptweave-classification-proposals-v3. If authenticated Zotero-origin evidence becomes a product requirement, a canonical transport adapter that actually observes the response must mint the attestation. A publicValueconstructor or caller-selected digest domain cannot grant that authority.Canonical crate-root repair
Review
5192569298found a structural reconciliation defect:[lib] path = "src/merged_lib.rs"made the canonical Research Intakesrc/lib.rsa nested module even though that file owns crate-levelforbid(unsafe_code),deny(missing_docs), and conditional nightly coverage-feature attributes. The wrapper also introduced a second crate-root seam around #9's canonical owner.RED
18e99a5982c326cf9460db590341bf58b735510aaddstests/library_root_contract.rs, requiring the package to retain canonicalsrc/lib.rsand that root to own/exportgolden_set. Production repaireccfc978fc28f883cfe548eb14e8eab715e3e4bfis one ordinary commit ahead / zero behind the RED and changes only three paths: removes the three-line Cargo lib-path override, addsmod golden_set; pub use golden_set::*;to the canonical root, and deletessrc/merged_lib.rs. No trusted aggregate field, public Zotero item shape, coverage threshold, provider/model policy or semantic authority changed.Documentation successors
9026fa4b60153fcb3a075b4ce99efcd3997d701e(CHANGELOG),f00abccaf95baa56400db872cb64a79d2e4a41bc(TRD), and current headbf6ad9a1c613bd4a36a237d914030997d5721b02(gap baseline) make that owner boundary code-current.Exact-head acceptance boundary
All predecessor Rust/fmt/Clippy/rustdoc/release/coverage evidence is historical and does not transfer after the crate-root repair. This execution host has no
cargo,rustc,rustfmt, orrustup, so no native RED/GREEN execution is claimed for this exact head. The RED is an executable regression contract, not a claim that it was run here.Keep this PR Draft. Require one unchanged exact head to pass repository-pinned Rust 1.98 workspace tests, fmt, strict all-target Clippy, warnings-denied rustdoc, release, owned production function/region/branch coverage, applicable Product/security/dependency/review checks, qualifying independent review, and zero valid unresolved findings before Ready or merge.
Central owner boundary
ConceptWeave protected/default
mainremainsf4f440dd58c77d7cd90dff8a1eb2eeb9a9940425. Product bootstrap #35 remains OPEN / Ready at9bb82f041483cb4e0cf1aa1f5450b413309f9a05; its Product PR workflow is not yet on protected main.Protected
.github/mainremains828eaaefb0cc97bba4da63eb9270447476d26710. Review-repair.github#2170@ae0f2f57f1d2abda7bb2e7ac9ce3bf8f1cac6f39remains OPEN / Ready / mergeable atSOURCE_REPAIRED / FULL_SUITE_DEPENDENCY_REPAIRED / ACCEPTANCE_PENDING. Agent Review Runtime Quality CI34775874685, Security Scan34775874680, Python Security34775874736, and SAST Semgrep34775874812are terminal success; CodeQL PR34775874728remains queued and no qualifying approval is present. Central CodeQL handler.github#2106@1ba96e4ddf6a800435651ec1c49acff533242fd9and Noema owner.github#2079@6d7e833224e06b4316df3d6bbdfcb4658e151956remain separate acceptance-pending prerequisites. Central evidence does not transfer to this leaf.Descendant handoff
#11 remains exact
6dff8c2ee42cfeb7bf8688c1f7e95989b61be266, OPEN / Draft and non-mergeable because its named parent branch has advanced beyond historical #10. Do not restack #11 while #10 is still acceptance-pending. After this exact #10 lineage reaches unchanged-head acceptance, #11 and later research descendants must ordinary/non-force adopt the complete accepted parent while preserving their own valid deltas and resetting exact-head evidence.Do not flatten/copy selected files, close valid descendants, force-push, destructively rebase, self-approve, dismiss review, weaken gates, synthesize status, use manual/no-op triggers, mutate Zotero, publish semantic truth or transfer predecessor evidence.