Skip to content

fix(signals): carry labelPolicy.note through focusManifestPolicyToCompilerOutput - #6020

Merged
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
luciferlive112116:fix/onboarding-pack-label-policy-note
Jul 15, 2026
Merged

fix(signals): carry labelPolicy.note through focusManifestPolicyToCompilerOutput#6020
loopover-orb[bot] merged 1 commit into
JSONbored:mainfrom
luciferlive112116:fix/onboarding-pack-label-policy-note

Conversation

@luciferlive112116

Copy link
Copy Markdown
Contributor

Summary

Closes #5943

Two adapters compile the same FocusManifestPolicy into the same RepoPolicyCompilerOutput.labelPolicy shape, and disagreed on one field:

  • compileRepoPolicyCompilerOutput (src/signals/repo-policy-compiler.ts) — the direct onboarding-pack API route + MCP tool path — sets note: labelPolicyNote(policy.publicSafe.validation.linkedIssuePolicy).
  • focusManifestPolicyToCompilerOutput (src/signals/onboarding-pack.ts) — used by buildRegistrationReadiness's onboardingPackPreviewomitted note entirely.

The adapter's own doc comment says it adapts a compiled policy "into the RepoPolicyCompilerOutput shape expected by buildRepoOnboardingPackPreview", so the two should agree for the same manifest. Because it didn't, sanitizeLabelPolicy received note: undefined and emitted null — so the registration-readiness report (surfaced to repo owners evaluating whether to register) always showed labelPolicy.note: null, silently losing the linked-issue guidance (e.g. "Link a tracked issue before opening a pull request.") that the very same manifest produces via the direct API/MCP path.

Fix: export the existing labelPolicyNote helper and reuse it as-is from the adapter. Per the issue's first suggested approach (the more minimal diff): the reverse import in repo-policy-compiler.ts is type-only and erased at compile time, so this introduces no runtime cycle. The helper's three linkedIssuePolicy branches are unchanged and not duplicated, and no other field mapping in focusManifestPolicyToCompilerOutput is touched.

Scope

Validation

  • git diff --check clean.
  • npm run typecheck — exit 0.
  • Focused suites green: onboarding-pack, registration-readiness, repo-policy-compiler, policy-sanitizer, self-dogfood-registration-pack, focus-manifest763 tests passed. These are the complete blast radius: focusManifestPolicyToCompilerOutput has exactly one caller (registration-readiness.ts:174), and every test file importing buildRegistrationReadiness is included.
  • Patch coverage verified against coverage/lcov.info for the exact changed lines: labelPolicyNote's body is covered (hits 30/23/18) and focusManifestPolicyToCompilerOutput is covered (hits 10). No uncovered changed lines.
  • Rebased onto current main.

New tests:

  • labelPolicy.note parity across both adapters for all three linkedIssuePolicy values (required / preferred / optional) — asserts a non-null note that is byte-identical to compileRepoPolicyCompilerOutput's, covering every labelPolicyNote branch.
  • An invariant that every other labelPolicy field is untouched (the fix is scoped to note).
  • End-to-end via buildRegistrationReadiness: onboardingPackPreview.labelPolicy.note is no longer always null.

If any required check was skipped, explain why:

  • Full test:ci not run end-to-end locally (Linux-only steps on Windows; the repo's full suite is not reliable under this environment). The change-relevant gates — typecheck, the complete affected-suite set, and per-line patch coverage — were validated directly and are green.
  • ui:openapi:check reports stale on a clean checkout of main in this environment (pre-existing, unrelated to this diff, which changes a runtime value and no schema), so no regenerated artifact is included — deliberately avoiding unrelated churn.

Safety

  • No secrets, wallets, hotkeys, trust scores, rewards, private rankings, or private maintainer evidence.
  • No auth/cookie/CORS/GitHub App/session change (pure in-memory adapter over an already-compiled, public-safe policy).
  • Public-surface safe: the note still flows through sanitizeLabelPolicysafeOptionalPublicText, and the text is the identical string the direct onboarding-pack path already publishes. registration-readiness's forbidden-public-language assertions remain green.
  • No API/OpenAPI schema change (labelPolicy.note already exists in the type and schema; only its runtime value is corrected).
  • No UI changes; no changelog edit.

…pilerOutput

Two adapters compile the same FocusManifest into the same
RepoPolicyCompilerOutput.labelPolicy shape and disagreed on one field:
compileRepoPolicyCompilerOutput (repo-policy-compiler.ts) sets
note: labelPolicyNote(linkedIssuePolicy), while
focusManifestPolicyToCompilerOutput (onboarding-pack.ts) omitted note
entirely.

The adapter's own doc comment says it produces the shape expected by
buildRepoOnboardingPackPreview, so the two should agree for the same
manifest. Because it didn't, buildRegistrationReadiness's
onboardingPackPreview.labelPolicy always rendered note: null -- silently
dropping the linked-issue guidance ("Link a tracked issue before opening
a pull request.") that the same manifest produces via the direct
onboarding-pack API/MCP path.

Export the existing labelPolicyNote helper and reuse it as-is rather than
duplicating its strings; the reverse import in repo-policy-compiler.ts is
type-only and erased, so no runtime cycle is introduced. No other field
mapping is touched.

Closes JSONbored#5943
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@codecov

codecov Bot commented Jul 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.16%. Comparing base (a211791) to head (3993268).
⚠️ Report is 19 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6020   +/-   ##
=======================================
  Coverage   95.16%   95.16%           
=======================================
  Files         591      591           
  Lines       46934    46934           
  Branches    15000    15000           
=======================================
  Hits        44664    44664           
  Misses       1512     1512           
  Partials      758      758           
Flag Coverage Δ
shard-1 43.92% <ø> (-0.01%) ⬇️
shard-2 36.55% <ø> (+0.02%) ⬆️
shard-3 31.92% <ø> (-0.08%) ⬇️
shard-4 31.90% <ø> (-0.06%) ⬇️
shard-5 32.24% <ø> (-0.30%) ⬇️
shard-6 44.92% <ø> (+0.34%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/signals/onboarding-pack.ts 98.90% <ø> (ø)
src/signals/repo-policy-compiler.ts 92.30% <ø> (ø)

@loopover-orb

loopover-orb Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Tip

🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-15 07:19:07 UTC

4 files · 1 AI reviewer · no blockers · readiness 80/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This is a small, well-targeted parity fix: it exports the existing labelPolicyNote helper from repo-policy-compiler.ts and reuses it in focusManifestPolicyToCompilerOutput (onboarding-pack.ts), so registration-readiness's onboardingPackPreview now surfaces the same non-null labelPolicy.note that the direct API/MCP path already produced. The root cause (a field omitted in one of two adapters compiling the same manifest shape) is fixed at the correct layer rather than patched downstream in sanitizeLabelPolicy, and the added tests assert parity between both adapters plus direct coverage in registration-readiness.test.ts across all three linkedIssuePolicy branches. The import direction (onboarding-pack.ts importing from repo-policy-compiler.ts) is a plain value import of a function, not type-only as the PR description claims, so it is worth confirming no circular-import runtime issue exists.

Nits — 5 non-blocking
  • The PR description asserts the import in onboarding-pack.ts is 'type-only and erased at compile time,' but the diff shows a regular value import (`import { labelPolicyNote } from './repo-policy-compiler'`), not a `import type` — worth correcting the description or the import for accuracy.
  • The doc comment added above labelPonlicyNote in repo-policy-compiler.ts (src/signals/repo-policy-compiler.ts:112) references onboarding-pack.ts by file path in a comment, which can drift if the file is renamed; a `@​see` or plain prose note would be more resilient.
  • The new test block in onboarding-pack.test.ts (test/unit/onboarding-pack.test.ts) duplicates the `GENERATED_AT` timestamp already possibly present elsewhere in the file — consider hoisting shared fixtures if this pattern recurs.
  • Double check for a real circular import between repo-policy-compiler.ts and onboarding-pack.ts at runtime (module load order), since the import is not actually type-only despite the PR claiming so.
  • Consider consolidating the duplicated adapter logic (two functions producing the same labelPolicy shape) into a single shared mapper in a follow-up, since this bug class (fields diverging between two adapters) is likely to recur otherwise.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #5943
Related work ⚠️ 2 scoped overlaps Top overlaps are listed below; lower-confidence bulk is hidden.
Change scope ❌ 8/20 High review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 109 registered-repo PR(s), 55 merged, 35 issue(s).
Contributor context ✅ Confirmed Gittensor contributor luciferlive112116; Gittensor profile; 109 PR(s), 35 issue(s).
Gate result ✅ Passing No configured blocker found.
Improvement ✅ Minor risk: clean · value: minor · LLM: moderate
Linked issue satisfaction

Addressed
The PR exports labelPolicyNote and calls it in focusManifestPolicyToCompilerOutput to populate labelPolicy.note, exactly matching the fix approach the issue suggested, and adds both the cross-adapter parity test and the end-to-end registration-readiness test the issue required.

Review context
Contributor next steps
  • Start here: Review top overlaps.
  • Then work through the remaining 2 steps in the Signals table above.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @loopover ask &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat &lt;question&gt; answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://gittensory.aethereal.dev/docs/loopover-commands

🧪 Experimental — new and may change.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.

  • Re-run LoopOver review

@loopover-orb loopover-orb Bot 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.

LoopOver approves — the gate is satisfied and CI is green.

@loopover-orb
loopover-orb Bot merged commit 79bfa91 into JSONbored:main Jul 15, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(signals): focusManifestPolicyToCompilerOutput drops labelPolicy.note, unlike its sibling adapter

1 participant