Skip to content

fix(labels): exempt outside-app labels from the drift report - #343

Merged
zengchuang-hw merged 4 commits into
InferMatrixLab:mainfrom
Dong1017:claude/label-drift-external-labels
Nov 5, 2026
Merged

zengchuang-hw merged 4 commits into
InferMatrixLab:mainfrom
Dong1017:claude/label-drift-external-labels

Conversation

@Dong1017

@Dong1017 Dong1017 commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator

Problem

The nightly safety-net job has failed on main since 2026-11-02, on its read-only label-drift step only:

label drift: bot-reviewed: in the repository, missing from .github/labels.json

Runs 41466562686 (11-02) and 41646825739 (11-03) report the same line. Both frozen-bytes jobs pass, and the 11-03 run passed lint, 3,544 tests and the evidence checks, so this is not a test regression.

bot-reviewed belongs to an outside GitHub App:

  • agent-ci-infra-bot[bot] created it and applies it; it is on 24 Issues and PRs.
  • A code search over this repository returns no reference to the name, and no workflow writes labels.
  • Its description is null and its colour is the API default, so it was never configured here.

Neither obvious fix works. Renaming or deleting it makes the app recreate bot-reviewed on its next review, so the drift returns the following night. Adding it to labels.json fails that file's own contract: manifest_problems requires every entry to be <group>:<value> with a matching group field (tools/check_labels.py:26-31), and bot-reviewed is ungrouped. docs/agent-collaboration.md separately forbids duplicating review or check states with labels, which is exactly what this label records.

Change

Record outside-owned labels in a separate file and exempt those names from the one report they cannot satisfy.

  • .github/external-labels.json (new): each entry is {"name", "owner"}, naming the app that owns the label. labels.json keeps its structure and its rules unchanged.
  • tools/check_labels.py: external_problems() validates the new file — exact fields, nonempty name, an owner that names an app, no duplicates, and no overlap with labels.json. drift() takes the external names and excludes them from "in the repository, missing from labels.json".
  • docs/agent-collaboration.md: states what the file is for, that these names are exempt from group:value and from the drift report because this repository neither creates them nor decides their wording, that label maintenance leaves them alone, and that they carry no coordination meaning here.

Three properties are deliberately preserved:

Property Why it still holds
group:value naming The rule still governs every label in labels.json; external labels are declared outside it, not admitted into it
No review/check state in labels The prohibition still governs this repository's own labels; bot-reviewed is not one of them
Drift detection The mechanism stays. A label that is neither listed nor declared external still fails

An Issue template or tools/skill_eval_*_config.json reference to an external label still fails, because this repository must not depend on a label it does not own. test_drift_still_rejects_a_template_reference_to_an_external_label covers that.

Validation

Run against this branch's files, with the repository's real 22 labels fetched from the labels API:

Check Result
pytest tests/test_check_labels.py 21 passed (16 existing, 5 new)
ruff check tools/check_labels.py tests/test_check_labels.py All checks passed
New checker, real 22 labels exit 0, no output
Old checker, same 22 labels exit 1, the exact nightly line — confirms the reproduction
New checker, bot-reviewed absent exit 0 — the app owns whether it exists
New checker, plus an unlisted stray-label exit 1 — genuine drift still fails

The five new tests cover a valid external list, malformed entries, duplicates, overlap with labels.json, an external label present or absent in the repository, and the template-reference rejection above.

Not run: the nightly workflow itself, and no label was created, renamed or deleted. This changes no labels, automation settings or acceptance criteria.

Scope

This fixes the drift report. It records no disposition for bot-reviewed beyond "an outside app owns it", and it does not by itself complete the label-consistency criterion in #284; the acceptance decision stays with that Issue's owner.

The nightly safety-net job has failed since 2026-11-02 because the live
`bot-reviewed` label is missing from `.github/labels.json`. That label belongs
to an outside GitHub App: nothing in this repository references it, no workflow
applies it, and renaming or deleting it only makes the app recreate it. Its name
also cannot enter `labels.json`, whose entries must be `group:value`.

Record such labels in `.github/external-labels.json` with the app that owns each
one, and exclude those names from the "in the repository, missing from
labels.json" report. A template or `skill_eval_*_config.json` reference to one
still fails, because this repository must not depend on a label it does not own.

Signed-off-by: Dong1017 <55723162+Dong1017@users.noreply.github.com>
Agent-assisted-by: Claude Code / claude-opus-5 (author)
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-05T06:58:30.165851Z 992eae4 New commits
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@Dong1017 Dong1017 left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implementation review: no blocking code finding at 4a0a17d352a7ebd683f7a7268b5567e4b3851210. The external-label policy disposition remains pending in #284.

Reviewed all four changed files against main 663f3b327998526a13aa31e72ad729025c18983c, the checker’s callers, relevant regression tests and hosted CI.

  • The exemption affects only unexpected live labels. Missing managed labels, managed-description drift and unlisted template/config references still fail. The separate list is checked for duplicate names and overlap with the managed manifest before its names reach drift().
  • Tests cover external-label presence and absence, malformed declared entries, overlap/duplicates and rejection of a template reference to an external label. The nightly invocation still uses the same read-only CLI; this change performs no label mutation.
  • Refreshed the head, conversation and review threads before posting. No existing inline findings require resolution; the automated review is complete on this head.

All six Ready jobs and the declaration check passed. The inspected Python 3.11 log reports Ruff success and 3,552 passed / 13 skipped / 284 subtests. Tested merge c60ee680 has no file differences from this head.

Remaining disposition: #284’s recorded owner or Issue-line delegate still needs to decide how this exception satisfies the accepted naming/live-label criteria. This review does not make that policy decision or clear the Issue’s other completion criteria.

Static inspection and existing hosted evidence only: I did not rerun tests, replay the author’s live-label snapshot or run a natural nightly. This is a non-voting COMMENT, not formal approval, merge authorization or Issue acceptance.

Copy link
Copy Markdown
Collaborator

Review at head 4a0a17d3: the checker change itself looks consistent (exemption only affects unexpected live labels; missing managed labels, description drift and template/config references still fail), but I am not approving yet because of an open policy question:

  1. Policy disposition still pending in task: agent-maintained issue system: one label scheme, auto-labeling and a pending-decision view #284: docs/agent-collaboration.md:226-233 adds a new category of labels exempt from the group:value rule and from the drift report. task: agent-maintained issue system: one label scheme, auto-labeling and a pending-decision view #284's owner/delegate has not yet recorded acceptance of this exception against its naming/live-label criteria. Please link that decision (or keep this PR open until it exists).
  2. Question (.github/external-labels.json:4): owner is free text (agent-ci-infra-bot[bot]) and is never checked against anything. Is it intended to be informational only? If so, please say so in the doc paragraph; if not, please describe how it is verified.
  3. Minor (tools/check_labels.py, external_problems): a non-string name (e.g. a list) is reported but then still flows into the name set used for duplicate/overlap checks; please confirm unhashable values cannot crash the check, or add a negative test.

@Dong1017 Dong1017 added the area:process Repo-wide process, governance, privacy and security rules label Nov 4, 2026 — with ChatGPT Codex Connector
Review found that `external_problems` reported a non-string `name` and then
still fed it to the duplicate and overlap sets, so a list or dict name raised
`TypeError: unhashable type` instead of printing a drift line. Only a usable
name now joins those comparisons, and three negative cases cover it.

Also state in the docs that `owner` is required but informational: no check
verifies it and none can, because GitHub does not report which app created a
label. Readers should confirm from the label's event history before relying on
an entry.

Signed-off-by: Dong1017 <55723162+Dong1017@users.noreply.github.com>
Agent-assisted-by: Claude Code / claude-opus-5 (author)
@Dong1017

Dong1017 commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator Author

Thanks — finding 3 was a real defect. Pushed 00b96cd4; all three points below.

3. Unhashable name (fixed). It did crash. Reproduced against the previous head:

>>> check.external_problems([{"name": ["a", "b"], "owner": "app"}], [])
TypeError: unhashable type: 'list'

external_problems appended the problem line and then let the same value reach the duplicate and overlap sets, so the check died on the way to printing it. Now only a usable name joins those comparisons:

if isinstance(name, str) and name:
    names.append(name)
else:
    problems.append(f"{name}: name must be a nonempty string")

test_an_unhashable_or_nonstring_external_name_is_reported_not_raised covers a list, a dict and an int; each returns one problem line and raises nothing. I also dropped the now-pointless key=str from the duplicate sort, since names holds only strings. A None name is added to the existing malformed-entry cases.

2. owner is informational, and the docs now say so. It is required and never checked, because it cannot be: GitHub does not report which app created a label, and the app's configuration is outside this repository. The doc paragraph now states that it is the reviewer's attribution of which app applies the label, that no check verifies it, that an entry is evidence of a past decision rather than a verified fact, and that a reader should confirm from the label's own event history before relying on one. Adding an entry stays a reviewed scope decision.

1. Policy disposition. Correct that it is outstanding, and I will not self-clear it. The requester decided the approach on 2026-11-04 — exempt the label rather than rename or delete it — on this reasoning, which I verified before building the change:

  • agent-ci-infra-bot[bot] created bot-reviewed and applies it; it is on 24 Issues and PRs. A code search returns no reference in this repository, no workflow writes labels, and its null description and default colour show it was never configured here.
  • So renaming or deleting it makes the app recreate it, and the drift returns the next night. Adding it to labels.json fails that file's own <group>:<value> contract.

I will record that disposition in #284 against its naming and live-label criteria, and link it here, before asking anyone to approve. Holding this PR open until that link exists is the right order — it is the same reason I have not treated the existing review as merge clearance.

Re-verified at 00b96cd4, against the repository's live 22 labels: 25 tests pass (21 plus the 4 new), ruff clean, the new checker exits 0 with no output, the pre-change checker still exits 1 with the exact nightly line, an absent bot-reviewed exits 0, and an unlisted stray-label still exits 1. No label was created, renamed or deleted, and the nightly workflow itself was not run.

Copy link
Copy Markdown
Collaborator

Incremental review 4a0a17d3..00b96cd4 (one commit, fix(labels): report an unusable external name instead of raising), checked against my earlier review at 4a0a17d3. Not approving yet, only because of item 1.

Resolved in this delta

  • 3. Unhashable / non-string name (tools/check_labels.py, external_problems, L159-179): only a nonempty str now joins names, so the duplicate (names.count) and overlap (set(names) & listed) comparisons can no longer see a list/dict. test_an_unhashable_or_nonstring_external_name_is_reported_not_raised (list, dict, int) and the added None case in test_invalid_external_entries_are_reported cover it, and each expects exactly one problem line. Dropping key=str from the duplicate sort is consistent with that.
  • 2. owner: docs/agent-collaboration.md L232-238 now states it is a required, informational attribution that no check verifies, and tells readers to confirm from the label's event history. That answers my question.
  • CI at 00b96cd4 is green (full-check and installed-package on 3.10/3.11, declarations, pr-references, kv-task-qualification).

Still open

  1. Policy disposition in task: agent-maintained issue system: one label scheme, auto-labeling and a pending-decision view #284: no comment on task: agent-maintained issue system: one label scheme, auto-labeling and a pending-decision view #284 since this push. As you said, please record the disposition against task: agent-maintained issue system: one label scheme, auto-labeling and a pending-decision view #284's naming / live-label criteria and link it here; I will re-review once that link exists.

Optional nits (not blocking)

  • external_problems assumes every entry is a dict: a bare string or null in .github/external-labels.json would raise AttributeError at entry.get("name") (L163) instead of being reported. If manifest_problems already guards this for labels.json, mirroring that here would keep the two files symmetric.
  • A whitespace-only name (" ") passes the isinstance(name, str) and name check, while owner uses .strip(). Consider name.strip() for consistency.

Static review of the delta plus hosted CI status; I did not run anything locally.

Review noted that `name` accepted `"  "` while `owner` was already checked with
`.strip()`. A whitespace-only name can never match a GitHub label, so it was
silently admitted as an exemption that exempts nothing. Check it the same way.

Signed-off-by: Dong1017 <55723162+Dong1017@users.noreply.github.com>
Agent-assisted-by: Claude Code / claude-opus-5 (author)

Copy link
Copy Markdown
Collaborator

Incremental review 00b96cd4..9451f08b (one commit, fix(labels): reject a whitespace-only external label name), checked against my earlier review at 00b96cd4. Not approving yet; details below.

Resolved in this delta

  • Whitespace-only name (tools/check_labels.py L50, external_problems): the check is now isinstance(name, str) and name.strip(), matching how owner is validated, so " " is reported and kept out of the duplicate/overlap sets. The new ({"name": " ", "owner": "a"}, "nonempty string") case in tests/test_check_labels.py L62 covers it.
  • Policy disposition (my item 1): recorded on task: agent-maintained issue system: one label scheme, auto-labeling and a pending-decision view #284 in this comment, addressing the naming and live-label criteria and scoping the exception to outside-owned labels. Thanks for linking it back to this PR.

Still open before I can approve

  1. CI at 9451f08b is not finished: full-check (3.10), full-check (3.11), installed-package (3.10) and kv-task-qualification were still in progress when I looked (pr-references, installed-package (3.11), declarations passed). I will re-check once they complete.
  2. Question on the disposition: the task: agent-maintained issue system: one label scheme, auto-labeling and a pending-decision view #284 record says it was made "under the Issue-line delegation recorded for this Issue" by the same account that authored this PR. Could you point to where that delegation is recorded, or confirm that the decision owner has seen it? I am not disputing the reasoning, I just want the approval to rest on an independently accepted policy decision rather than a self-recorded one.

Optional nit (still not blocking)

  • external_problems (L44) still calls entry.get("name") before checking the entry is a dict, so a bare string or null in .github/external-labels.json would raise AttributeError instead of being reported.

Static review of the delta plus hosted check status; I did not run anything locally.

@Dong1017

Dong1017 commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator Author

Policy disposition recorded. Your item 1 is answered: #284 issuecomment-6631265140 records the disposition against that Issue's criteria 1 and 2, and #284's status block now points at it with Decision needed: none.

In short: the group:value rule governs the labels this repository manages — the ones it can create and rename — and criterion 2 is read as managed labels matching the manifest exactly, with outside-owned labels declared and excluded. The record also states what it does not do: it endorses neither the label nor the app's reviews, leaves the review-state prohibition intact for every managed label, and lets no label be exempted by assertion. It names the alternative that would supersede it — removing the app's label-write access, or having it adopt a group:value name — in which case this PR should be closed rather than merged.

Your two nits. One taken, one I think rests on a wrong premise.

Whitespace-only name: fixed in 9451f08b. You were right that it was inconsistent. I checked it first:

>>> check.external_problems([{"name": "  ", "owner": "app"}], [])
[]

A whitespace-only name cannot match a GitHub label, so it was being admitted as an exemption that exempts nothing. name.strip() now matches owner's check, with " " added to the malformed-entry cases.

Non-dict entry: I would rather leave it. The premise does not hold — manifest_problems does not guard this either. Both raise the same way today:

>>> check.manifest_problems(["kind:bug"])
AttributeError: 'str' object has no attribute 'get'
>>> check.external_problems(["kind:bug"], [])
AttributeError: 'str' object has no attribute 'get'

So the two files are already symmetric, and guarding only the new one would break that. Both manifests are tracked files that change through review, so a bare string in either is a malformed-file problem rather than a drift case; if it is worth reporting, it is worth reporting for both, which is a change to existing behaviour and belongs in its own PR rather than this one. Happy to do it either way if you prefer — say which and I will follow.

Re-verified at 9451f08b: 26 tests pass, ruff clean, the new checker exits 0 against the live 22 labels, and an unlisted stray-label still exits 1.

@Dong1017

Dong1017 commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator Author

Item 1 is resolved: CI finished at 9451f08b and every check is green. Nothing changed; the four jobs you saw in progress completed a few minutes later.

Check Result Run
pr-references, installed-package (3.10/3.11), full-check (3.10/3.11), kv-task-qualification SUCCESS 41837125609
declarations SUCCESS 41837122050
knowledge-model-policy SUCCESS 41838851621

Item 2: the delegation is not recorded in a repository artifact. I would rather say that plainly than point you at something that only resembles one.

What does exist:

  • The decision owner for this Issue line is the account holder behind Dong1017. The exemption approach was chosen by that account holder on 2026-11-04, in the request that led me to open this PR; the alternative on the table was removing the app's label-write access or having it adopt a group:value name. I implemented that choice and wrote it up. I did not make it.
  • The earliest public trace is the D1–D4 decision record of 2026-11-01, which states the same arrangement for this Issue line. It is itself written by an acting agent, so it does not answer your concern on its own.

Your concern holds regardless: this account covers both the decision owner and the agents acting for them, so nothing posted from it can be the independent acceptance you are asking for.

@zengchuang-hw — you are already a requested reviewer here and a repository admin. Would you accept or reject the policy reading recorded in #284 issuecomment-6631265140? The question is narrow: does group:value govern only the labels this repository can create and rename, so that a label owned by an outside app is declared and excluded from the drift report rather than renamed into the manifest? An answer from an account other than this one is what makes the acceptance independent, and this PR needs a second approval in any case.

If that independent acceptance does not come, the right outcome is to close this PR rather than merge it, and to take the alternative the disposition names.

The optional nit stays unchanged, for the symmetry reason in my previous comment: manifest_problems raises the same AttributeError on a bare string today, so guarding only the new function would make the two asymmetric. If you would rather have the guard, I will add it to both in a separate PR.

@Dong1017
Dong1017 requested a review from david6666666 November 4, 2026 14:58
@Dong1017

Dong1017 commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator Author

@david6666666 — requesting your review here as well.

What the PR does: bot-reviewed is created and applied by an outside GitHub App (agent-ci-infra-bot[bot]), so the repository cannot rename or delete it, and its name does not fit the group:value rule that .github/labels.json enforces. The nightly safety-net has therefore failed on its label-drift step for three days running, today included (run 41842010370); every other nightly job passes. This PR adds .github/external-labels.json, validates it, and excludes only those declared names from the drift report. Missing managed labels, description drift and unlisted template references still fail.

State: head 9451f08b, 4 files changed, all 8 checks green. @amy-why-3459 has reviewed three increments and found one real defect, which is fixed; her remaining blocker is not about the code.

The part that needs someone other than me. The policy reading behind this PR is recorded in #284 issuecomment-6631265140: that group:value governs the labels this repository can create and rename, so a label owned by an outside app is declared and excluded rather than renamed into the manifest. I wrote both that record and this PR from the same account, so it cannot be an independent acceptance no matter how it is phrased. @zengchuang-hw was asked first and has not replied yet.

Either form of help is useful: accept or reject that policy reading, or review the code on its own terms. If the reading is rejected, the right outcome is to close this PR and instead remove the app's label-write access or have it adopt a group:value name — that alternative is named in the disposition and I would act on it the same way.

@Dong1017

Dong1017 commented Nov 5, 2026

Copy link
Copy Markdown
Collaborator Author

@amy-why-3459 @zengchuang-hw can you review again?

Copy link
Copy Markdown
Contributor

Re-review at 9451f08b9c4f72d664f616e9c6f2aa369fe78495, against the previous incremental review. The head is unchanged. I found no blocking implementation issue. The policy confirmation remains open.

Resolved

  • Whitespace-only and non-string external names are rejected before duplicate and overlap checks. The exception is limited to unexpected live labels with declared names. Missing managed labels, managed description drift, and references to external labels from templates or config still fail. Checker
  • The current-head Ready jobs and knowledge policy check passed. I also ran the 26 label-checker tests on Windows at this head, and Ruff and the diff whitespace check passed.

Before I can recommend approval

  • Please have an independent maintainer record in #284 whether the outside-app exception is accepted for the naming and live-label criteria, then link that confirmation here. The existing disposition and this implementation come from the same account, and the request for independent confirmation is still open.

This is a non-voting re-review of the checker, its tests, and the linked records. The post-merge nightly verification remains part of the Issue follow-up.

Validate objects before field access in both label manifests, and keep unusable names out of duplicate and overlap comparisons. Add symmetric validation and CLI regressions.

Agent-assisted-by: dot (implementation)
Signed-off-by: Codex <codex@openai.com>

Dong1017 commented Nov 5, 2026 •

Copy link
Copy Markdown
Collaborator Author

@amy-why-3459 @zengchuang-hw please re-review new head 992eae4210f1a03ff5e134b7a80f2b008a3691d4.

  • Addressed Amy's remaining malformed-entry nit in both manifests: string/null/scalar/list entries are reported before field access, and invalid entries or names cannot crash the duplicate/overlap checks. The existing managed-label, description and template-reference checks remain intact.
  • Added 14 regression cases: all fail against the previous implementation and pass with the fix. The focused suite is 40 passed. Full local validation on Python 3.12 is 3,572 passed, 12 skipped, 284 subtests passed; repository-wide Ruff and the diff whitespace check passed.
  • All 8 current-head checks passed. Ready CI: full-check and installed-package on Python 3.10/3.11, pr-references and kv-task-qualification. Declarations and knowledge-model-policy also passed. CI verifies this implementation; it does not settle the policy decision below.

The independent-maintainer policy confirmation requested in zengchuang's review is still pending. Please include an independent acceptance/rejection and link it from #284's disposition if you can provide that decision. This code fix does not clear that blocker or complete #284; post-merge nightly verification remains outstanding.

Copy link
Copy Markdown
Collaborator

Incremental review 9451f08b..992eae4210f1 (one commit, fix(labels): report malformed manifest entries without crashing), checked against my earlier review at 9451f08b. Not approving yet.

Resolved in this delta

  • Non-dict / non-object entries (tools/check_labels.py manifest_problems L24-27 and external_problems L48-51): both now report entry {n}: expected an object and continue before any field access, so a bare string/null/list/scalar no longer raises AttributeError. Symmetric between the two manifests, as discussed.
  • Unusable managed names stay out of duplicate checks (manifest_problems L38-39): only dict entries whose name is a str join the duplicate scan. test_unusable_managed_names_do_not_reach_duplicate_checks covers list/dict/int/None.
  • Overlap/duplicate still reported alongside malformed entries (test_invalid_entries_do_not_hide_duplicates_or_overlap): a leading None plus duplicate/overlap cases still surfaces all three problem lines.
  • CLI path (test_main_reports_nonobject_entries_and_fails_without_drift): validation failure prints the object error + ADVICE and does not reach drift/reference reads.
  • Hosted Ready CI at 992eae4210f1 is green (full-check 3.10/3.11, installed-package 3.10/3.11, pr-references, kv-task-qualification, declarations).

Still open before I can approve

  1. Independent policy acceptance for the outside-app exception: the code side of my earlier findings is clear, but the task: agent-maintained issue system: one label scheme, auto-labeling and a pending-decision view #284 disposition and this implementation still come from the same account. Please have an independent maintainer accept or reject the reading in #284 issuecomment-6631265140 (and link that confirmation here), as zengchuang also asked. I am not clearing that blocker from this review.

Static review of the delta plus hosted check status; I did not run the suite locally.

zengchuang-hw commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Incremental review 9451f08b..992eae4210f1 (one commit), against the previous review. Current head is 992eae4210f1a03ff5e134b7a80f2b008a3691d4. No blocking implementation finding in this delta. Independent policy acceptance remains outstanding.

本轮已解决

  • Both manifests reject non-object entries before field access. Managed-name duplicate checks and external/managed overlap checks filter unusable entries and names, while valid duplicates and overlap are still reported alongside malformed entries. Checker
  • The CLI returns failure with the validation diagnostics before drift or template-reference reads. I ran all 40 label-checker tests successfully at this head. Running the 14 added regression cases against the parent checker produced 14 failures, confirming that they exercise the reported defects. Regression tests
  • Current-head full-check (3.10/3.11), installed-package (3.10/3.11), pr-references and kv-task-qualification passed. The knowledge policy status is also successful. Ready CI · Knowledge policy

批准前必做(Before I can recommend approval)

非阻塞问题或建议

No additional finding in this delta.

Checks and limits

Validation ran on Windows with Python 3.12.1, pytest 9.1.1 and Ruff 0.15.15 at the head above. Targeted pytest, Ruff and the delta whitespace check passed. I also checked the CLI callers and the target advance through 6fefa010. That advance does not change this checker, its tests, dependencies or Ready workflow. The current collaboration/review rules were re-read. The full suite evidence here comes from hosted CI. Actual post-merge nightly verification and the remaining #284 acceptance criteria remain Issue follow-up.

Codex performed this non-voting incremental review through zengchuang-hw, under the account holder's direct delegation to review and reply to #343.

@Dong1017

Dong1017 commented Nov 5, 2026

Copy link
Copy Markdown
Collaborator Author

@zengchuang-hw can you review again, since amy-why-3459 has approved this PR (a kind of agreement for #284).

zengchuang-hw commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

PR review verdict — PASS for the bounded software change at 992eae4210f1a03ff5e134b7a80f2b008a3691d4.

Same-head re-review: 992eae4210f1a03ff5e134b7a80f2b008a3691d4 → 992eae4210f1a03ff5e134b7a80f2b008a3691d4; target main, reviewed against e843b9d31cb86e1fd12d00890c89798b0bedd1fe. This answers the re-review request and updates the preceding review.

Correction: I withdraw the extra requirement for a separate independent policy-confirmation comment. #284's recorded delegation and the authorized disposition already provide the scope for this change. I had incorrectly turned a request for additional clarification into a mandatory approval gate.

本轮已解决

  • The implementation preserves managed-label naming/description checks, unknown-label drift and template/config-reference checks while excluding only declared external names from unexpected-live-label drift.
  • Malformed entries and unusable names are handled before field access, duplicate and overlap checks. The previous exact-head verification passed 40 tests, with all 14 added regression cases failing against the parent implementation.
  • Current-head Ready CI and knowledge policy pass. No new code delta or regression finding is present.

批准前必做(Before I can approve)

None. No unresolved blocking finding remains for this PR's content. No additional policy vote, clarification comment or test rerun is required by this review.

非阻塞问题或建议

No additional finding. Post-merge nightly observation and whole-Issue completion remain the existing Issue follow-up.

Actual checks and limits

Rechecked the exact head, existing diff/review evidence, current checks, original authorization and disposition. A separate Codex agent independently reassessed that authority evidence. Reused the recorded tests because the code head is unchanged; no new local tests or live-label mutations were performed.

The account holder has now directly authorized formal approval of reviewed PR content when no unresolved blocker remains. The formal review will be bound to this exact head.

@zengchuang-hw zengchuang-hw left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

PR review verdict — PASS

Reviewed head 992eae4210f1a03ff5e134b7a80f2b008a3691d4 targeting main at e843b9d31cb86e1fd12d00890c89798b0bedd1fe. Same-head follow-up to the preceding review; the corrected re-review withdraws the extra policy-confirmation gate.

本轮已解决

  • The scoped external-label exception follows the recorded Issue-line delegation and authorized disposition.
  • The checker keeps managed naming/description, unknown-drift and template/config-reference enforcement. Both manifests report malformed entries before field access; invalid names do not crash duplicate/overlap checks.
  • The exact-head technical review recorded 40 passing label-checker tests and 14 regressions that fail against the parent implementation, plus Ruff and whitespace checks. Current-head Ready CI and knowledge policy are successful.

批准前必做(Before I can approve)

None. No unresolved blocking finding remains in the reviewed PR content.

非阻塞问题或建议

No additional finding. Post-merge natural-nightly observation and whole-Issue completion remain the recorded Issue follow-up.

Actual checks and limits

Rechecked the unchanged head, target, original authority/disposition, earlier diff/caller/test evidence and current checks. A separate Codex agent independently reassessed the authority evidence. No new local test run, live-label mutation or nightly replay was performed for this approval.

Delegation and execution: directly delegated by the account holder zengchuang-hw, who authorized content-based formal approval when no unresolved blocker remains. Executor: Codex, acting as independent PR reviewer through the connected GitHub account zengchuang-hw. Scope: this PR's software content and supporting evidence at the exact head above. Authorized action exercised: formal Approve, bound with commit_id to that head. Merge, label changes and Issue closure are separate actions.

@zengchuang-hw
zengchuang-hw merged commit 86f8f4a into InferMatrixLab:main Nov 5, 2026
8 checks passed
@Dong1017
Dong1017 deleted the claude/label-drift-external-labels branch November 5, 2026 12:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:process Repo-wide process, governance, privacy and security rules

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants