docs(outdated): clarify monorepo tag pattern matching for virtual subdirectory deps#1895
Conversation
3597689 to
a28768f
Compare
There was a problem hiding this comment.
Pull request overview
Updates apm outdated documentation (and reinforces behavior with a regression test) for monorepo tag-pattern matching when dependencies are installed as virtual subdirectory packages via path:.
Changes:
- Document that
{name}in patterned git tags for virtual subdirectory deps is derived from thepath/virtual_pathbasename (not the repo name), and add a concreteapm.ymlexample. - Add a sample-output row illustrating a monorepo virtual subdirectory dependency.
- Add a unit test covering monorepo patterned-tag resolution for a virtual subdirectory dependency, and update
_package_basename()to use the virtual path basename for tag-pattern matching.
Reviewed changes
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/apm_cli/commands/outdated.py |
Adjusts package-name derivation for patterned tag matching to use virtual_path basename for virtual subdirectory deps. |
tests/unit/test_outdated_command.py |
Adds a regression test ensuring monorepo patterned tags are recognized for virtual subdirectory dependencies. |
docs/src/content/docs/reference/cli/outdated.md |
Clarifies monorepo tag pattern behavior for path: deps and adds example/sample output. |
…directory deps
- Document that {name} in patterned tags is derived from virtual_path
basename for virtual subdirectory packages (e.g. packages/my-pkg).
- Add example row showing monorepo virtual dep in sample output.
- Add concrete apm.yml example with path: for monorepo deps.
Follow-up to microsoft#1504 and microsoft#1893.
a28768f to
448777a
Compare
kevinbeier-enbw
left a comment
There was a problem hiding this comment.
Both Copilot nits addressed in amended commit:
- Clarified that tag names use the final path segment only (, not ).
- Fixed extra leading space in sample output table.
kevinbeier-enbw
left a comment
There was a problem hiding this comment.
Both Copilot nits addressed in amended commit:
- Clarified that tag names use the final path segment only (my-pkg_v1.2.3, not packages/my-pkg_v1.2.3).
- Fixed extra leading space in sample output table.
APM Review Panel:
|
| Persona | B | R | N | Takeaway |
|---|---|---|---|---|
| Python Architect | 0 | 1 | 0 | Doc claims virtual_path drives {name} tag-pattern inference; _package_basename reads repo_url only -- described monorepo tag resolution is not implemented. |
| CLI Logging Expert | 0 | 2 | 1 | New sample row misaligns Current column; aspirational {name} claim contradicts _package_basename(); pre-existing count wrong. |
| DevX UX Expert | 0 | 1 | 2 | Doc/code mismatch: {name} in tag pattern comes from repo URL basename (monorepo), not path: segment (my-pkg), so the documented example silently fails. |
| Supply Chain Security Expert | 0 | 1 | 1 | Doc-only PR is safe; one recommended note on resolved_commit as integrity anchor; one nit on path validation mention. |
| OSS Growth Hacker | 0 | 1 | 1 | Strong monorepo adoption signal from community, but doc describes code behavior that does not exist -- aspirational docs erode trust faster than missing docs. |
| Doc Writer | 3 | 1 | 1 | PR documents virtual-path tag derivation that the code does not implement: _package_basename uses repo_url, not virtual_path; all added content is factually wrong. |
B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.
Top 5 follow-ups
- [Doc Writer] (blocking-severity) Remove all three sections encoding the false claim: description prose at line 21, sample output row at line 60 (uses my-pkg_v* tags the code cannot produce), and YAML example section at lines 83-92 ('tag name derived from virtual path'). -- Runtime-verified:
infer_tag_pattern('my-pkg_v1.0.0', 'monorepo') = None. The described pipeline does not execute. Users who configure a monorepo tag scheme based on this doc get silently wrong outdated status. - [Python Architect] Open a companion code PR adding an
is_virtualguard in_package_basename():if dep.is_virtual and dep.virtual_path: return dep.virtual_path.rstrip('/').split('/')[-1]. -- This is the implementation that makes the documented behavior true. The fix is ~3 lines and unblocks the doc resubmit, turning a community contribution into a shipped enterprise feature. - [OSS Growth Hacker] After the code companion merges, resubmit this doc PR against the updated codebase with a CHANGELOG entry: 'apm outdated now correctly resolves tag patterns for monorepo subdirectory packages via path: segment'. -- Provides a clear landing path for the contributor and completes the enterprise adoption story.
- [Supply Chain Security Expert] Add a sentence noting that
apm.lock.yamlrecords the resolved commit SHA (not the tag name) as the actual integrity anchor. -- Makes the trust model explicit for enterprise users. Fold into the resubmitted doc PR. - [CLI Logging Expert] During the resubmit pass, fix the pre-existing Current column overflow (9-char column, several rows exceed it) and update '[!] 2 outdated deps found' to match the actual count in the sample table. -- Monospace table alignment is the primary rendering contract for CLI reference docs.
Architecture
classDiagram
direction LR
class LockedDependency {
<<Dataclass>>
+repo_url str
+virtual_path str
+is_virtual bool
+marketplace_plugin_name str
+resolved_ref str
+get_unique_key() str
}
class OutdatedRow {
<<ValueObject>>
+package str
+current str
+latest str
+status str
+source str
}
class PackageBasename {
<<Pure>>
+call(dep) str
}
note for PackageBasename "Reads dep.repo_url only. dep.virtual_path is never consulted. Returns 'monorepo' for a dep at org/monorepo with path: packages/my-pkg. Fix: guard on is_virtual."
class ResolveTagPattern {
<<Pure>>
+call(ref str, name str) str
}
class CheckOneDep {
<<IOBoundary>>
+call(dep dl verbose) OutdatedRow
}
class InferTagPattern {
<<Pure>>
+call(tag str, pkg_name str) str
}
CheckOneDep ..> LockedDependency : reads
CheckOneDep ..> PackageBasename : calls
CheckOneDep ..> ResolveTagPattern : calls
CheckOneDep ..> OutdatedRow : produces
PackageBasename ..> LockedDependency : reads repo_url only
ResolveTagPattern ..> InferTagPattern : delegates
class PackageBasename:::touched
class LockedDependency:::touched
classDef touched fill:#fff3b0,stroke:#d47600
flowchart TD
A["apm outdated"] --> B
B["_check_one_dep(dep)"] --> C
C["[I/O] list_remote_refs"] --> D
D["_package_basename(dep)"] --> E
E{marketplace_plugin_name?}
E -- yes --> F["return marketplace_plugin_name"]
E -- no --> G["repo_url.split('/')[-1]\nreturns 'monorepo' NOT 'my-pkg'"]
G --> H["_resolve_tag_pattern('my-pkg_v1.0.0', 'monorepo')"]
H --> I["infer_tag_pattern with name='monorepo'"]
I --> J["DEFAULT_TAG_PATTERNS: monorepo_v.. / monorepo--v.. / monorepo-v..\nNO MATCH for 'my-pkg_v1.0.0'"]
J --> K["infer_tag_pattern returns None"]
K --> L["TAG_RE.match: FAIL on 'my-pkg_v1.0.0'"]
L --> M["tag_pattern=None, is_tag=False"]
M --> N["_find_remote_tip('my-pkg_v1.0.0')"]
N --> O["no branch named 'my-pkg_v1.0.0'"]
O --> P["status=unknown, source=git branch\nDOCS CLAIM: outdated, git tags -- FALSE"]
style P fill:#ffcccc,stroke:#cc0000
Recommendation
Hold PR #1895 until the three aspirational sections are removed or replaced with accurate current-behavior prose. The fastest path to merge: open the companion code PR adding the _package_basename is_virtual guard, then amend this PR to reflect post-fix behavior. Give kevinbeier-enbw the exact rework checklist (lines 21, 60, 83-92) and a direct pointer to the code change needed -- their third contribution in this area is a high-value enterprise signal and deserves a warm, actionable response rather than a silent close.
Full per-persona findings
Python Architect
- [recommended] _package_basename ignores virtual_path; documentation describes behavior that does not exist in current code at
src/apm_cli/commands/outdated.py:53
LockedDependency.virtual_path (lockfile.py:76) carries the path: segment for virtual subdirectory deps. _package_basename(dep) at outdated.py:53-60 reads dep.repo_url and returns its final URL segment; it never checks dep.is_virtual or dep.virtual_path. For a dep with repo_url='org/monorepo' and virtual_path='packages/my-pkg', _package_basename returns 'monorepo'. _resolve_tag_pattern then calls infer_tag_pattern('my-pkg_v1.0.0', 'monorepo'): all DEFAULT_TAG_PATTERNS fail (name prefix is wrong -- 'monorepo' vs 'my-pkg'), returning None. TAG_RE at line 76 also fails. tag_pattern=None, is_tag=False, and the dep falls to _find_remote_tip yielding OutdatedRow(status='unknown', source='git branch'). The documentation at outdated.md:21 states '{name} is derived from the final path segment' and the sample output at line 60 shows 'outdated, git tags' -- both unreachable with current code.
Suggested: Add virtual_path branch before the repo_url fallback in _package_basename:if dep.is_virtual and dep.virtual_path: return dep.virtual_path.rstrip('/').split('/')[-1]
CLI Logging Expert
- [recommended] Doc claims {name} derives from virtual path segment, but _package_basename() uses repo_url basename at
docs/src/content/docs/reference/cli/outdated.md:21
_package_basename() at outdated.py:53-60 returns repo_url.rstrip('/').split('/')[-1]. For dep with repo_url='org/monorepo', yields 'monorepo', not 'my-pkg'. infer_tag_pattern('my-pkg_v1.0.0', 'monorepo') fails silently -- the dep falls through without tag-pattern detection. The documented behavior is aspirational, not current.
Suggested: Either gate the prose on the feature landing, or replace with accurate current behavior: 'For virtual subdirectory packages, {name} is currently derived from the repository basename. Use an explicit marketplace tag_pattern to override.' - [recommended] New sample output row overflows Current column: 'my-pkg_v1.0.0' (13 chars) exceeds the 9-char column defined by the separator at
docs/src/content/docs/reference/cli/outdated.md:60
The static code block uses a fixed-width table. The new row's Current value overflows by 4 chars, pushing subsequent columns right and breaking the monospace alignment.
Suggested: Widen the Current column separator, shorten the example ref to 'v1.0.0'/'v1.1.0', or annotate the block as approximate Rich output. - [nit] Pre-existing: '[!] 2 outdated dependencies found' count does not match; sample table shows 7 outdated rows including the new one at
docs/src/content/docs/reference/cli/outdated.md:66
Suggested: Update to '[!] 7 outdated dependencies found' or reduce the sample table to fewer rows.
DevX UX Expert
- [recommended] _package_basename() uses repo URL basename, not virtual_path segment -- the documented tag-inference example does not work at
docs/src/content/docs/reference/cli/outdated.md:21
For a dep with path: packages/my-pkg, the PR documents that {name} resolves to 'my-pkg' so tags like my-pkg_v1.2.3 are matched. But _package_basename returns 'monorepo' (repo URL last segment). infer_tag_pattern fails. The dep is NOT processed via tag comparison at all -- users following this doc get silent wrong behaviour with no actionable error.
Suggested: Do not merge the description sentence until _package_basename() is updated to return dep.virtual_path.rstrip('/').split('/')[-1] when dep.virtual_path is set. - [nit] Sample output row overflows the Current column at
docs/src/content/docs/reference/cli/outdated.md:60
'my-pkg_v1.0.0' is 13 characters; Current column separator is 9 dashes. - [nit] Monorepo apm.yml example sits between two flag-invocation examples with no heading at
docs/src/content/docs/reference/cli/outdated.md:83
Suggested: Add a#### Monorepo subdirectory packagesheading immediately before the YAML block.
Supply Chain Security Expert
- [recommended] Example does not clarify that resolved_commit SHA -- not the tag name -- is the actual integrity anchor at
docs/src/content/docs/reference/cli/outdated.md:84
The YAML example pins viaref: my-pkg_v1.0.0, a mutable git tag. APM records the resolved_commit (40-char SHA) in apm.lock.yaml at lock time. Without this note, the example implies weaker guarantees than APM actually provides.
Suggested: Add a comment:# apm.lock.yaml records the commit SHA resolved at lock time; the tag ref drives outdated detection only -- not the integrity pin. - [nit] 'derived from the final path segment' does not note that path values are validated (traversal sequences rejected) at
docs/src/content/docs/reference/cli/outdated.md:21
Suggested: Change to 'derived from the final path segment (path values are validated; traversal sequences are rejected)'.
OSS Growth Hacker
- [recommended] Aspirational docs for this enterprise use-case are the highest-risk trust-erosion pattern -- ship the code first at
docs/src/content/docs/reference/cli/outdated.md:21
_package_basename() returns 'monorepo' (last URL segment), not 'my-pkg'. The result is tag_pattern=None, is_tag=False, apm outdated silently misclassifies the dep. Shipping this directs enterprise monorepo users to a path that appears broken, destroying the trust this PR is trying to build.
Suggested: Hold until companion code fix lands, or reframe as a known limitation with workaround: 'Set an explicit tag_pattern in the marketplace entry to override.' - [nit] Sample output row columns misalign at
docs/src/content/docs/reference/cli/outdated.md:60
Doc Writer
- [blocking] Description bullet claims {name} comes from path: segment -- contradicts _package_basename() which reads repo_url at
docs/src/content/docs/reference/cli/outdated.md:21
_package_basename(dep) at outdated.py:53-60 returns repo.rstrip('/').split('/')[-1], where repo is dep.repo_url. It never reads dep.virtual_path. For dep with git: .../monorepo.git and path: packages/my-pkg, the function returns 'monorepo', not 'my-pkg'. The added sentence is false. Users who configure their monorepo tag scheme as my-pkg_v* based on this doc get incorrect outdated status.
Suggested: Remove the appended sentence entirely until the code change to _package_basename() that reads virtual_path lands. - [blocking] Sample output row uses my-pkg_v* tags the current code cannot produce; also overflows the Current column by 4 chars at
docs/src/content/docs/reference/cli/outdated.md:60
Under current implementation, _package_basename returns 'monorepo', so the tag pattern would match monorepo_v*, not my-pkg_v*. Additionally 'my-pkg_v1.0.0' (13 chars) overflows the 9-char Current column.
Suggested: Remove this row entirely. If re-added, use tag names matching the repo-URL-derived pattern and fit within column width. - [blocking] YAML example section and its prose describe unimplemented behavior -- the exact scenario shown silently fails at
docs/src/content/docs/reference/cli/outdated.md:83
Lines 83-92 add 'Monorepo dependency installed via path: (tag name derived from virtual path)' and claim 'With a newer tag my-pkg_v1.1.0 on the remote, apm outdated reports it as outdated.' This is false: the code would not match my-pkg_v1.1.0. Since the code change is absent, this section must be removed.
Suggested: Remove lines 83-92. Re-add as a complete unit once the code change lands and is tested. - [recommended] Word count increased ~90 words with no compensating removal at
docs/src/content/docs/reference/cli/outdated.md
The PR adds 14 lines without removing anything. Even if correct, additions trend word count up without a compensating cut. - [nit] The YAML snippet structure itself is valid (git:, path:, ref: fields are correctly formed) and can be reused once the code lands.
Auth Expert -- inactive
PR #1895 modifies only docs/src/content/docs/reference/cli/outdated.md -- no auth, token management, credential resolution, host classification, or remote-host fallback code is touched.
Test Coverage Expert -- inactive
documentation-only PR -- no runtime code paths to defend
Performance Expert -- inactive
documentation-only PR -- no performance-relevant source files modified
This panel is advisory. It does not block merge. Re-apply the
panel-review label after addressing feedback to re-run.
Generated by PR Review Panel for issue #1895 · 548 AIC · ⌖ 9.23 AIC · ⊞ 5.5K · ◷
- Widen Current column in sample output so monorepo tag names fit. - Fix outdated dependency count in sample output (7, not 2). - Add #### Monorepo subdirectory packages heading before YAML example. - Add note that apm.lock.yaml stores resolved_commit SHA, not the tag, as the integrity anchor. - Mention path validation / traversal-rejection in the virtual_path prose.
Head branch was pushed to by a user without write access
…al, fix heading level Remove security implementation detail from patterned-tag bullet: the path-validation note belongs in internals docs, not a usage reference bullet. All six active panelists flagged the same sentence. Fix heading level inconsistency: the new Monorepo subdirectory section used h4 (####) while the adjacent Status values section uses h3 (###). Both are subsections of Examples; align to h3. Also trim redundant parenthetical from the subsection intro sentence: tag derivation is already explained in the Description section above. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
APM Review Panel: ship
cc @kevinbeier-enbw -- a fresh advisory pass is ready for your review. All six active panelists converged on the same signal: the inline parenthetical in the patterned-tag bullet ("(path values are validated; traversal sequences are rejected)") is an implementation detail that does not belong in a usage-focused reference bullet. Doc-writer additionally identified that the new "#### Monorepo subdirectory packages" section used h4 while the adjacent "### Status values" section uses h3 -- a heading-level inconsistency. Both items touched only the file this PR already modifies and were folded in this pass. The supply-chain expert raised a concern about Dissent. Supply-chain expert raised an accuracy concern about Growth signal. The patterned-tag bullet is the highest-traffic entry for monorepo consumers. A shorter, cleaner bullet removes unnecessary cognitive load for the common case. Panel summary
Top 3 follow-ups
RecommendationItems 1 and 2 were folded in this pass (both touched only the file this PR already modifies, within stated scope). Item 3 deferred as scope boundary crossed. CI is expected to remain green. The documentation is accurate and the PR is ready for maintainer review. Full per-persona findingsPython Architect
CLI Logging Expert
DevX UX Expert
Supply Chain Security Expert
OSS Growth Hacker
Doc Writer
Auth Expert -- inactiveDocumentation-only PR; no auth surface modified. Test Coverage Expert -- inactiveDocumentation-only PR -- no runtime code paths to defend. Performance Expert -- inactiveDocumentation-only PR; no performance-relevant source files modified. This panel is advisory. It does not block merge. Re-apply the |
What
Follow-up documentation for PR #1504 (monorepo tag support) and PR #1893 (virtual subdirectory fix).
Changes
docs/src/content/docs/reference/cli/outdated.md:path:inapm.yml), the{name}in patterned tags is derived from the final path segment ofvirtual_path— not the repo name.org/monorepo/packages/my-pkg my-pkg_v1.0.0 my-pkg_v1.1.0 outdated).apm.ymlexample withpath: packages/my-pkgandref: my-pkg_v1.0.0.Why
Before this docs update, users installing monorepo subdirectories had no way of knowing that the tag pattern matching uses the
pathbasename. This led to confusion whenapm outdatedreportedunknownfor valid monorepo tag layouts.Refs: #1504 #1893