Skip to content

fix(review): mark gate.firstTimeContributorGrace as reserved/inert#2411

Merged
JSONbored merged 1 commit into
mainfrom
claude/first-time-grace-inert-docs
Jul 1, 2026
Merged

fix(review): mark gate.firstTimeContributorGrace as reserved/inert#2411
JSONbored merged 1 commit into
mainfrom
claude/first-time-grace-inert-docs

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

What

.gittensory.yml.example documents gate.firstTimeContributorGrace: true as softening a would-be BLOCK to advisory for a genuine newcomer. The value is faithfully parsed, clamped, and threaded end-to-end through RepositorySettings into GateCheckPolicy at every call site — but evaluateGateCheckCore (src/rules/advisory.ts) never reads policy.firstTimeContributorGrace or its authorMergedPrCount/authorClosedUnmergedPrCount companions anywhere in its body. A nearby code comment confirms this was a deliberate removal ("blocker findings must remain closure/rejection outcomes"); an existing test already asserts a newcomer with grace=true still gets conclusion: "failure". The public docs were simply never updated to match.

This is not a guard weakening — the actual behavior is stricter than advertised — but it's a real config-as-code correctness gap: a maintainer's mental model of their own gate's leniency toward newcomers is wrong.

Fix

  • Rewrote the .gittensory.yml.example entry, the RepositorySettings.firstTimeContributorGrace doc in types.ts, and the GateCheckPolicy field docs (all three: the flag plus its two companion counts) in advisory.ts to state the setting is reserved/currently inert, rather than describing softening behavior that no longer runs.
  • Added a manifest-parse warning (FocusManifest.warnings) when a repo explicitly sets gate.firstTimeContributorGrace: true, so a maintainer relying on it gets a visible signal instead of silent inertness. Unset or explicit false — the default, and also the effective runtime behavior — does not warn.

The field itself is kept (not deleted): the design comment frames the removal as deliberate-but-field-retained, and a warning needs a field to warn about.

Tests

  • New test: setting gate.firstTimeContributorGrace: true produces a manifest warning matching "reserved/inert".
  • New test: leaving it unset, or explicitly false, produces no such warning.
  • npx tsc --noEmit clean.
  • npx vitest run test/unit/focus-manifest.test.ts — 138/138 pass.
  • Diff-range coverage check: all three changed source files fully covered, zero gaps.
  • Regression sweep across focus-manifest/predicted-gate/gate-check-policy/settings-preview/api suites — 315/315 pass.
  • Full unsharded npm run test:coverage — 309 files / 5685 tests pass.
  • npm audit --audit-level=moderate — 0 vulnerabilities.

Advances #1936. Closes #2266.

evaluateGateCheckCore in src/rules/advisory.ts never reads
policy.firstTimeContributorGrace (or its authorMergedPrCount /
authorClosedUnmergedPrCount companions) — a deliberate removal ("blocker
findings must remain closure/rejection outcomes") that the public-facing
docs never caught up with. A maintainer reading .gittensory.yml.example
would believe setting this true softens a blocker for newcomers; nothing
changes, and a first-time contributor with a real blocker is still
one-shot closed like anyone else.

- Rewrite the .gittensory.yml.example entry, the RepositorySettings field
  doc in types.ts, and the GateCheckPolicy field docs in advisory.ts to
  state the flag (and its two companion counts) are reserved/inert rather
  than describing behavior that no longer runs.
- Emit a manifest warning when a repo explicitly sets
  gate.firstTimeContributorGrace: true, so a maintainer relying on it gets
  a visible signal instead of silent inertness. Unset/false (the default,
  and also the effective behavior) does not warn.

This is not a guard weakening — the actual behavior was already stricter
than documented. Only the docs and warning are new.
@dosubot dosubot Bot added the size:M label Jul 1, 2026
@codecov

codecov Bot commented Jul 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.78%. Comparing base (a8030ca) to head (a9b7b34).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2411   +/-   ##
=======================================
  Coverage   95.78%   95.78%           
=======================================
  Files         224      224           
  Lines       24917    24919    +2     
  Branches     9056     9057    +1     
=======================================
+ Hits        23867    23869    +2     
  Misses        428      428           
  Partials      622      622           
Files with missing lines Coverage Δ
src/rules/advisory.ts 97.64% <ø> (ø)
src/signals/focus-manifest.ts 99.18% <100.00%> (+<0.01%) ⬆️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@loopover-orb

loopover-orb Bot commented Jul 1, 2026

Copy link
Copy Markdown

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-01 20:04:38 UTC

5 files · 1 AI reviewer · no blockers · readiness 75/100 · CI green · clean

⏸️ Suggested Action - Manual Review

  • Touches a guarded path — held for manual review

Review summary
The diff correctly limits the change to documentation plus manifest feedback: the runtime gate behavior is untouched, the public example/type comments now describe `firstTimeContributorGrace` as inert, and `parseFocusManifest` warns when the gate flag is explicitly true. The added tests cover the true warning path and the false/unset non-warning paths. No blocking runtime defect is visible in the provided diff; the main remaining risk is incomplete cleanup of older documentation surfaces.

Nits — 6 non-blocking
  • nit: `migrations/0039_first_time_contributor_grace.sql:1` still documents the old softening behavior, so one repository-visible source continues to advertise the stale contract.
  • nit: `src/signals/focus-manifest.ts:440` adds the warning on the `gate.firstTimeContributorGrace` path, but I could not verify from the provided full files whether a generic `settings.firstTimeContributorGrace: true` manifest path can set the same inert value silently.
  • Update or clearly annotate `migrations/0039_first_time_contributor_grace.sql:1` if this repository treats migration comments as current docs, or add a pointer to the current inert behavior elsewhere that supersedes it.
  • If `settings.firstTimeContributorGrace` is still accepted in manifests, mirror the warning there and add a focused test next to `test/unit/focus-manifest.test.ts:797`.
  • PR author also opened the linked issue — Link an issue that was opened by a different contributor, or provide a rationale for why this self-authored issue represents genuine discovery work.
  • Touches a guarded path — held for manual review — A maintainer must review and merge this change.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #2266
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: 67 registered-repo PR(s), 57 merged, 589 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 67 PR(s), 589 issue(s).
Gate result ⚠️ Not blocking Advisory; not blocking this PR.
Review context
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Review top overlaps.
  • Add a concise scope and risk note.
  • Triage stale or unlinked PRs.
  • No action.
  • Check active issues and PRs before submitting.
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.

🟩 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 Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@loopover-orb loopover-orb Bot added gittensor gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. labels Jul 1, 2026
@JSONbored JSONbored self-assigned this Jul 1, 2026
@JSONbored
JSONbored merged commit 6ecacec into main Jul 1, 2026
12 checks passed
@JSONbored
JSONbored deleted the claude/first-time-grace-inert-docs branch July 1, 2026 22:13
@github-project-automation github-project-automation Bot moved this from Todo to Done in gittensory - v1 roadmap Jul 1, 2026
@github-actions github-actions Bot mentioned this pull request Jul 2, 2026
12 tasks
loopover-orb Bot pushed a commit that referenced this pull request Jul 12, 2026
#5321)

firstTimeContributorGrace is explicitly RESERVED/inert (#2266) --
the gate evaluator never reads it. #2411 already fixed the
config-as-code half of this exact problem: a parse-time warning plus
a rewritten .gittensory.yml.example correctly disclose inertness.
But #2411 never touched the dashboard/API surface -- the live
maintainer-dashboard toggle still read "Soften a newcomer's block to
advisory" with zero disclosure, the one surface a non-technical
maintainer is most likely to actually use.

Applies the same disclose-don't-remove treatment #2411 gave the yml
surface: the toggle now renders greyed-out and non-interactive with
a "Reserved -- currently has no effect (#2266)" hint (new `disabled`
prop on ToggleControl/ToggleFieldDef), and the same disclosure is
added as a .describe() on all 3 API/openapi occurrences. Kept the
field itself in the schema (not removed) to match the #2411
precedent and avoid an API-breaking change for a field that is
genuinely harmless to still accept.

Regenerated openapi.json.

Refs #5292
loopover-orb Bot pushed a commit that referenced this pull request Jul 16, 2026
…-to-end (#6714)

Confirmed via #6446's two decisions: (1) the 7 gate:-only fields
(gatePack, slopGateMode/slopGateMinScore/slopAiAdvisory,
mergeReadinessGateMode, manifestPolicyGateMode,
linkedIssueSatisfactionGateMode, requireFreshRebaseWindowMinutes,
aiReviewLowConfidenceDisposition) are already correctly wired through
applyGateConfigOverrides and already have DB columns + dashboard UI, so no
new settings.* twins are added -- gate: stays canonical, matching #6444's
decision. (2) firstTimeContributorGrace has been RESERVED/INERT since
#2411 deliberately stopped evaluateGateCheckCore from reading it (blocker
findings must remain closure/rejection outcomes, not softened for a
newcomer) -- rather than wiring it in against that settled design, this
deletes the dead surface entirely: the DB column (migration 0161), the
manifest gate.firstTimeContributorGrace parsing in both the app and
engine copies of focus-manifest.ts, the predicted-gate engine mirror, the
dashboard's disabled disclosure toggle (#5321), and every doc/example
mention.

Closes #6446, closes epic #6440 (all four config-as-code batches now
shipped).
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

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

fix(review): gate.firstTimeContributorGrace is documented as softening blockers but is dead/inert

1 participant