You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
aggregate local-plugin commits into deterministic user-facing release topics while preserving every source ref
align standalone and weekly release-note item limits with the Doc Agent dynamic 12-18 policy
handle net-zero reverts and numeric hexadecimal commit refs correctly
make the read-only offline preview compact large evidence sets without truncating required refs
add v2.0.18 and future high-commit regression coverage
Why
The v2.0.18 release had 29 important local-plugin commits. The old drafting path produced 12 items that referenced only 12 of those commits, so strict evidence validation stopped the workflow before npm, tag, and GitHub Release side effects.
The Doc Agent already supports a dynamic item budget, but these caller-side MemOS changes had not been committed or submitted. This PR makes both sides use the same rule and groups related commits before drafting, so concise notes can still retain complete evidence coverage.
The <= 15 bound is inconsistent with what the implementation actually guarantees. releaseTopicsForCommits calls compactReleaseTopics with releaseTopicLimitForRequiredCount(commits.length) where commits.length is 31 (not 29), yielding a real limit of min(18, max(12, ceil(31/2))) = 16. The test-only commits are excluded from hints but still counted in commits.length, so the effective compact limit is 16, not 15. This test could spuriously fail if the actual topics land at 16. The assertion should use releaseTopicLimitForRequiredCount(commits.length) to stay in sync with the implementation.
The key-uniqueness check passes trivially because compactReleaseTopics always assigns a fresh consolidated-* key to every merged topic. This means the Set size will always equal compacted.length regardless of whether distinct source topics were incorrectly collapsed. The check does not verify that a topic's original key is preserved when it is not merged — only that the output keys are mutually distinct. Consider also asserting that topics below the compaction threshold retain their original key values.
These expected values (12, 15, 18) are hardcoded magic numbers that duplicate the formula min(MAX_RELEASE_TOPICS, max(MIN_RELEASE_TOPICS, ceil(n/2))) without referencing the named constants. If MIN_RELEASE_TOPICS or MAX_RELEASE_TOPICS changes in the production code, the test values silently become wrong with no hint of the correct values. Consider deriving the expected values from the exported constants or at least adding an inline comment with the formula.
This new code removes both filters, so revert commits will pass through fallbackTopicForText and may appear as release note entries. Consider re-adding the revert-commit guard:
MIN_RELEASE_ITEMS reads as "the minimum number of items required", but it is actually the floor for the dynamic cap computed in releaseItemLimitForRequiredCount. A name like RELEASE_ITEMS_LIMIT_FLOOR or MIN_RELEASE_ITEM_LIMIT would express the intent more clearly.
An unrecognised category value silently falls back to "Improved" copy, producing quietly incorrect release notes with no diagnostic. Consider logging a warning:
if(!copy[category]){warn(`combinedFallbackCopy: unknown category "${category}", falling back to Improved`);}returncopy[category]||copy.Improved;
The scoring formula is non-obvious: a lower score wins, so pairs with fewer combined source refs are merged first, keeping high-evidence items intact. Without a comment this reads like an arbitrary magic number and will mislead future maintainers. Consider adding:
// Prefer merging low-evidence pairs first (fewest combined source_refs) to preserve// high-evidence items as independent entries.
In the new topic-mode branch, refs are added to refToGroup but are no longer added to knownRefs. The old code (hint-mode) called knownRefs.add(ref) inside this loop. knownRefs is used downstream in coverageFromReleaseItems to determine which refs are "known" when computing coverage. With this omission, topic-mode refs won't appear in knownRefs, which could cause coverage checks to treat them as unknown and produce false coverage failures. Consider restoring knownRefs.add(ref) inside the topic-mode loop.
The removal of if (/^\d{2,}$/.test(text)) return \#${text}`;silently drops bare numeric PR refs (e.g."2311") — they now return ""instead of being normalized to"#2311"`. If any existing draft payloads or external callers emit bare numeric strings as source_refs, those refs will be lost without any warning. This is a silent data-loss breaking change. Verify that no upstream producer emits bare numeric refs, or add an explicit rejection path.
The count / 2 divisor (half the commit count, clamped to 12–18) has no explanation. It is non-obvious why halving the commit count is the right ratio, and a future maintainer who changes MIN_RELEASE_TOPICS or MAX_RELEASE_TOPICS may not realize the relationship. A short comment explaining the intent (e.g. "roughly one release topic per two commits") would help.
This backfill unconditionally appends all refs from owned groups onto the winning item, even refs that the item never originally referenced. While this preserves topic evidence coverage, it means a release item's source_refs in the output can contain refs it never had in the input. Downstream consumers that assume source_refs only includes refs the AI explicitly cited may be surprised. Consider documenting this behavior or validating that downstream processing handles the expanded ref list correctly.
The inner spread-and-reverse [...seen].reverse().find(...) creates a new reversed array on every revert commit, making the overall loop O(n²) in the number of commits. For typical release ranges this is fine, but if the commit range ever grows large (hundreds of commits with many reverts), this will become noticeable. A Map from SHA to commit would make lookups O(1).
The fallback in categoryHintForSubject changed from returning null ("let the AI decide") to returning a concrete "Improved" hint. This means every previously unrecognized commit now gets an opinionated nudge toward "Improved" rather than being freely classified. This is a deliberate policy shift that affects AI output quality. The change itself may be intentional, but a comment documenting why "Improved" is the right default would prevent future maintainers from reverting it without understanding the rationale.
These three assertions for releaseItemLimitForRequiredCount are embedded inside a test that is primarily about draft validation. If any of these assertions fail, the test aborts before the validation logic runs, producing a misleading failure message with no context. They should be extracted into their own dedicated unit test.
This exact-count assertion couples the test to the current output of releaseItemLimitForRequiredCount(29). If the scaling formula changes intentionally, this test breaks without a clear explanation. Consider using releaseItemLimitForRequiredCount(29) as the expected value to make the intent explicit:
Mutating process.env directly inside the test is not safe under parallel test runners. Another concurrent test could observe the modified environment between the mutation and the finally restore. If the test runner ever enables concurrency, this will cause non-deterministic failures. Consider running this test explicitly serially, or passing overrides as arguments to requestDocAgentDraft if the API allows it.
The test name says "keeps categories separate", but this assertion only verifies that no unexpected category values appear — it passes even if compaction silently drops all "Fixed" items. Add an explicit check that both categories are present in the output:
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
area:coreMOS 编排层 / 框架底座 / 跨模块问题status:readyReady for implementation; waiting for assignee or AI dispatch | 可进入实现,等待认领或派发
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Why
The v2.0.18 release had 29 important local-plugin commits. The old drafting path produced 12 items that referenced only 12 of those commits, so strict evidence validation stopped the workflow before npm, tag, and GitHub Release side effects.
The Doc Agent already supports a dynamic item budget, but these caller-side MemOS changes had not been committed or submitted. This PR makes both sides use the same rule and groups related commits before drafting, so concise notes can still retain complete evidence coverage.
Policy
min(18, max(12, ceil(required_count / 2)))source_refsare preservedSafety
Verification
node --checkpassed for both changed scriptsgit diff --checkpassedMemOS Release — Pre-Merge Dry Runpassed in run 33365598362