Skip to content

docs: audit Desktop and frontend references - #4810

Open
jackeyfaker77 wants to merge 1 commit into
apache:mainfrom
jackeyfaker77:codex/3522-desktop-frontend-audit
Open

docs: audit Desktop and frontend references#4810
jackeyfaker77 wants to merge 1 commit into
apache:mainfrom
jackeyfaker77:codex/3522-desktop-frontend-audit

Conversation

@jackeyfaker77

Copy link
Copy Markdown
Contributor

Summary

  • Audit all seven Desktop and frontend documents against the current renderer and @maka/ui implementation.
  • Keep the generated file-level inventory as the current authority, and mark four dated audit/fix logs as historical with fresh current-state pointers.
  • Synchronize the English and Chinese CSS governance documents with the current layer order, selector scopes, token ownership, and !important exception policy.
  • Reconcile the contradictory historical height classification and remove a duplicate audit row.

Refs #3522

Verification

  • npm run astryx:surface-inventory — 249 files, 0 blockers, 0 reimplementations, 1 polish item, 248 aligned.
  • npm run astryx:surface-inventory:test — 19/19 tests passed.
  • node apps/desktop/scripts/check-renderer-architecture.mjs — passed.
  • Changed-document ASF header validation — all six headers canonical.
  • Relative Markdown link and exact repository path checks — no missing targets.
  • git diff --check — passed.
  • npm run check:renderer-architecture — 97/98 fixture tests passed; the single failure is Windows path normalization of a POSIX-only /fixture/... fixture test input. The actual renderer architecture checker above passes.
  • Full-checkout ASF validation: pre-existing missing headers under untracked .bika-* staging directories are unrelated to this PR.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Documentation-only change; Biome ignores Markdown and no product runtime typecheck was required. All relevant generator and architecture checks pass as reported above.

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

@github-actions github-actions Bot added the effort/M Under 500 readable lines label Sep 4, 2026

@Astro-Han Astro-Han 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.

Reviewed current head 23dde4de5c1efa44b9d479b9639b26b7a1417059 (OPEN, MERGEABLE/BLOCKED awaiting human review). One P2 plus two P3s below, no P0/P1. Docs-only change (6 Markdown files, no code); checks green on this head. This is a high-quality audit — every checkable factual claim was independently verified and all numbers hit exactly.

P2 — the newly written !important enumeration is incomplete and will mislead auditors

The governance doc states as a closed enumeration ("remain in" three files) that narrow compatibility/product overrides live only in reference-shell.css, styles/settings/usage.css, and packages/ui/src/styles.css. A head-wide search finds 10 style files mentioning !important; after classifying (default exceptions, centralized cursor policy, comment-only mentions), two remain that are neither default exceptions nor in the three listed files: shell-layout.css (!important: beats SideNav's inline width) and sidebar.css (!important: StyleX childrenInner paddingInlineStart is unlayered). Graded P2 not P3 because the frontmatter positions this as implementation_status: current / document_status: stable — a living contract for contributors. A closed enumeration in a living contract that omits two items has concrete consequences: auditors following it will misjudge those files, or governance scripts built from the list will miss them. And this PR's entire purpose is doc/implementation alignment — this is the one place it fails its own goal.

P3s (non-blocking)

  • The 249 / 0 / 0 / 1 / 248 snapshot numbers are hardcoded in three docs (mitigated: each carries last_verified plus a pointer to re-run the inventory command — but a future change must touch three places with no check catching a miss; prefer keeping numbers only in the generated inventory).
  • translation_status: synced is self-declared with no machine enforcement (spot-checked: 9 headings each side, structurally aligned — but nothing flips the field when only one side changes; a limitation of the frontmatter mechanism, not this PR).

What I could not judge

npm run astryx:surface-inventory was not run — numbers were counted from the committed inventory file, so if that generated artifact is itself out of sync with disk, the verification follows it (the coverage gate owning that sync was not validated). Whether the rewritten CSS governance conventions are the ones the team wants is a frontend-owner call — only their stated accuracy was verified, not their desirability.


Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

简体中文

本条结论全部来自 @Opus-Qronos-AstroHan 的审查。我自己没有读这份 diff;我核的是当前 head 有没有漂移。当前 head 是 23dde4d,未关闭。审计质量很高数字全中,一条 P2:新写的封闭枚举漏了两处,会误导按它审计的人,另有两条小的快照与翻译提醒。等人类拍板。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/M Under 500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants