Skip to content

fix: address Greptile stack follow-ups and internal ViewModel boundary - #167

Open
MaxMichel2 wants to merge 8 commits into
chore/sample-tidy-audit-reportfrom
fix/greptile-stack-follow-ups
Open

MaxMichel2 wants to merge 8 commits into
chore/sample-tidy-audit-reportfrom
fix/greptile-stack-follow-ups

Conversation

@MaxMichel2

@MaxMichel2 MaxMichel2 commented Nov 9, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Addresses the ten approved outstanding Greptile findings across the existing stack, after checking all twelve inline comments. Two were already fixed and are recorded rather than reimplemented.

  • Retry superseded mock-load failures against fresh state without changing fail-closed, cancellation, probability/delay or sequence-commit guarantees.
  • Keep built-in NetworkMockViewModel internal rather than add a public screen recovery API; enforce that boundary with Konsist.
  • Keep error recovery buttons visible, measure rendered Unicode width, and retain comparison caches across unrelated recompositions.
  • Retain independent per-feature retries and reject superseded intentions, including On/Off/On writes.
  • Restore paused Console by retained row key/offset after saved noise preferences load; missing anchors remain paused at the newest matching row.
  • Bound TimeCapsule expansion state by retained entries, including eviction, Clear and hidden-screen restoration.
  • Correct the analytics README example and module identity documentation.

Breaking API change: the standalone public NetworkMockViewModel constructor/class is removed. Register the module for UI; use the core repositories for runtime integration. Public loggers and runtime configuration remain available.

Resolution record: anti-slop/greptile-stack-follow-ups-2026-11-09.md, linking every original comment and its evidence. This PR does not automatically resolve old threads or claim to include comments posted after the inventory snapshot.

Validation

  • 437 registered Android host tests, zero failures/errors/skips. Some unchanged tasks used Gradle cache; strengthened Ktor cases ran in the preceding focused run.
  • 210 root connected Android device tests, zero failures/errors/skips.
  • 15 Konsist tests, zero failures/errors/skips.
  • Detekt, updated API signature baseline, sample debug assembly and Windows-equivalent Dokka/versioned Zensical docs build passed.
  • Normal gitleaks and Detekt hooks passed on all seven commits.
  • Local AST graph updated and integrity-checked; no generated Graphify artifacts are tracked.

Tests cover stale exceptional/absent loads, cancellation, single probability/delay, atomic sequence advancement, independent/ABA feature writes, delayed-preference Console restoration, bounded live/saved expansion, offscreen wide Unicode, bounded error actions and comparison cache identity/invalidation.

Detekt KMP/generated-source analysis warnings remain qualified. API baseline success is not backward-compatibility approval for the intentional public removal. iOS runtime, exhaustive keyboard/reduced motion, arbitrary host themes, spoken TalkBack and further 200% font checks are not newly signed off. Earlier owner-approved visual audit evidence remains scoped. PR G's observed Actions blocker was a GitHub internal server error before jobs; no workflow edits or reruns were made, and local checks do not establish remote CI success.

Stack and merge order

PR H is based on G, not main. Merge from the bottom upward. Existing PR heads/bases are unchanged.

Layer PR Head Base
Original #157 feat/networkmock-ux-revamp main
A #159 refactor/devview-shell-ui-kit feat/networkmock-ux-revamp
B #160 refactor/featureflip-segmented-control refactor/devview-shell-ui-kit
C #161 feat/analytics-event-detail refactor/featureflip-segmented-control
D #163 refactor/console-log-rows feat/analytics-event-detail
E #164 fix/timecapsule-restore-target refactor/console-log-rows
F #165 refactor/networkmock-sheet-detail fix/timecapsule-restore-target
G #166 chore/sample-tidy-audit-report refactor/networkmock-sheet-detail
H This PR fix/greptile-stack-follow-ups chore/sample-tidy-audit-report

Native stack: https://github.com/worldline/devview/stack/162

RetriggerView in GreptileConfidence Score: 4/5

The PR appears safe to merge, with non-blocking follow-ups for crowded feature errors, preview startup work, and one documentation typo.

Findings

  1. P2 Failed saves hide controls ▶
  2. P2 Large previews delay opening ▶
  3. P2 Docs suggest an invalid constructor ▶

Summary

The PR fixes stale mock-load recovery, keeps the built-in NetworkMockViewModel internal, and strengthens saved UI state and retry behavior.

  • NetworkMock retries a failed mock load when the selected state has changed.
  • FeatureFlip keeps failed saves separate for each feature.
  • Paused Console returns to the saved log row after its filters load.
  • TimeCapsule keeps expanded rows only while their entries remain.
  • NetworkMock error messages scroll without hiding recovery actions.
  • Response preview panes size scrolling to the widest rendered line.
  • Response comparisons keep their cached preview during unrelated updates.
  • The built-in NetworkMock ViewModel is no longer public API.
  • The Analytics setup example uses module registration.

MaxMichel2 explicitly approved the public ViewModel removal. The supplied resolution record also treats spoken TalkBack and further 200% font checks as deferred; this review does not claim those checks passed. No builds, device tests, or visual captures were run during this review.

Reviews (1) · Last reviewed commit: ":memo: Record Greptile fixes and current..." · Reviewed by Greptile

Reserve recovery actions outside bounded scrolling errors, measure rendered
Unicode line widths, and retain comparison previews across unrelated updates.

Validation: complete root Android device suite passed 210 tests, including
bounded error actions, a wider offscreen Unicode line in a 5,000-line preview,
and comparison cache identity/invalidation. Host, architecture, lint, API
and sample build checks also passed; iOS runtime remains unverified.
Recheck exceptional and absent loads before reporting a stale selection failure.
Still-current errors remain fail-closed and cancellation still propagates.

Validation: 46 Ktor host tests passed, including stale mock/sequence replacements,
one roll/delay, atomic advancement and cancellation. Complete root host/device,
architecture, lint, API, sample and documentation checks passed.
Remove the standalone public constructor contract rather than expose screen
recovery internals. Registered UI retains its shared consumer and Retry/Cancel.
Require internal built-in ViewModels through Konsist and update API/docs.

Validation: 15 architecture tests, API signature check and full host/device
suites passed. The 17-line public signature removal is intentionally breaking.
Track failed choices per feature and distinguish newer intent revisions,
including On/Off/On. A new choice supersedes only its own older failure.

Validation: all 14 FeatureFlip device tests passed in the complete 210-test
run, including independent retries and suspended ABA writes. Host, lint,
architecture, API and sample build checks passed.
Resolve the saved row and pixel offset after noise preferences load. A missing
anchor selects the newest matching row at zero offset while remaining paused.

Validation: all 23 Console device tests passed, including disposed restoration,
delayed preferences and a missing anchor behind a newer excluded row. Complete
host/device, architecture, lint, API, sample and docs checks passed.
Prune live, saved and restored expansion IDs after eviction and Clear while
preserving recording identity. Hidden-screen restore cannot resurrect stale IDs.

Validation: all 12 TimeCapsule device tests passed, including live-state pruning
without saving, eviction, Clear and restoration. Complete host/device,
architecture, lint, API, sample and docs checks passed.
Replace the removed analytics provider example, document module identity and
record all twelve review dispositions with approved semantics and limitations.

Validation: 437 registered host, 210 device and 15 architecture tests passed;
lint, API baseline, sample build and versioned documentation builds passed.
The record distinguishes cached tasks and deliberately removed public API.
Remote CI success and exhaustive iOS/accessibility coverage are not claimed.
@MaxMichel2
MaxMichel2 added this pull request to stack #162 November 9, 2026 19:35
Comment on lines +193 to +196
failedChanges.forEach { (name, change) ->
key(name) {
Row(
modifier = Modifier.fillMaxWidth().padding(horizontal = 16.dp),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Failed saves hide controls

FeatureFlipScreen now puts one error row per failed feature above the list in a non-scrolling Column. If several queued saves fail, these rows can consume all available height and hide the feature list or later Retry buttons. Put the errors inside a scrolling list, or give the error area a bounded, scrollable height.

Comment on lines +360 to +368
val textWidth = remember(key1 = lines, key2 = textMeasurer, key3 = textStyle) {
lines.maxOfOrNull { line ->
textMeasurer
.measure(
text = line,
style = textStyle,
softWrap = false,
maxLines = 1
).size.width

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Large previews delay opening

ScrollableLinePane now measures every line before it can show a preview. A 5,000-line body needs 5,000 text measurements during composition, even though only a few rows are visible. This brings expensive work for offscreen rows back onto the UI thread. Keep the Unicode width fix, but calculate widths outside composition or spread the work across frames while keeping the pane usable.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Comment thread docs/modules/featureflip.md Outdated

## Module identity

`FeatureFlip()` supplies its built-in `name`, `subtitle` and `icon` for Home. Register the module

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Docs suggest an invalid constructor

The new identity section calls the module FeatureFlip(), but FeatureFlip is an object and cannot be constructed. Use FeatureFlip here, matching the registration example above.

Suggested change
`FeatureFlip()` supplies its built-in `name`, `subtitle` and `icon` for Home. Register the module
`FeatureFlip` supplies its built-in `name`, `subtitle` and `icon` for Home. Register the module

@MaxMichel2 MaxMichel2 self-assigned this Nov 10, 2026

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant