Plan: Variant lineage tree visualization (objective-colored, hover-preview, click-to-detail) (#123)#267
Merged
Merged
Conversation
f8306c2 to
5474fee
Compare
fc77cdc to
44be76b
Compare
…eview, click-to-detail) (#123) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Round-0/1 codex-review corrections incorporated: - Forgejo deep links use a distinct FORGEJO_WEB_URL browse-base (NOT the credential-bearing FORGEJO_CLONE_URL_HOST clone URL). - JSON embedding via dict + |tojson (not |safe on a string); client reads textContent. - Objective evaluator: two-class failure model — experiment-global (parse error / unknown OR non-numeric metric) → loud banner + grey-all; per-node gap → grey. First-metric fallback dropped from v0 (deferred). - Grammar narrowed to + - * / parens unary-minus; namespaced node ids; degenerate color-scale rule; multi-parent diff labeling. - Added automated live-stack assertion for /admin/lineage/ + JS assets; corrected wave-gate wording (interim subsets vs literal pre-push quartet). - Config-drift risk + operator-facing objective-grammar docs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Line 19 + 69: 'promotion candidate' — distinct semantic from the retired integrate-as-promote verb (here: future code-promotion to wider scope). Line 103: backticked code function name 'evaluate' — code reference, not the retired task-kind verb. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
44be76b to
fdf4128
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Plan-stage PR for #123 — codex-reviewed to convergence (3 rounds: 0→1→2).
Adds
docs/plans/issue-123-variant-lineage-tree.md— the implementation strategy for a read-only/admin/lineage/web-ui page rendering the experiment as an objective-colored variant-lineage DAG (D3 force-directed), plus the bundled Forgejo deep links on the variant-detail page. Full codex-review record committed underdocs/plans/review/issue-123/plan/.Key findings driving the plan
list_variants/read_variant/read_idea/read_experiment); variants carryparent_commitsdirectly. Matches the Redesign pending-task lists (executor + evaluator): high-signal columns, sort by priority, filter by eligibility, group by creator #137 / 12a-1c web-ui-module-only precedents.objective.expris opaque/implementation-defined and nothing in the reference impl evaluates it (eventarget_condition_policydeliberately uses a single metric). The plan builds a safeast-allowlist evaluator (nevereval()).app.state.base_commit_sha; the root renders colorless today and colors automatically once Evaluatable baseline variant (seed becomes a kind=baseline Variant) #122 lands.What codex-review changed (rounds 0–1, converged round 2)
FORGEJO_WEB_URLbrowse-base, NOT the credential-bearingFORGEJO_CLONE_URL_HOSTclone URL (would leak creds / malform links).textmetric) → loud invalid-objective banner + grey-all; per-node gap → that node grey. The issue's blanket "first-metric fallback" is deliberately not implemented (mixes incompatible semantics into one heat map) — deferred.+ - * /+ parens + unary minus (noPow/Mod); namespaced node ids (seed:<sha>/variant:<id>) to avoid opaque-variant_idcollision; degenerate color-scale rule; multi-parent diff link labeling.|tojson(not|safeon a string; XSS-safe), client readstextContent./admin/lineage/+ both JS assets, plus a corrected wave-gate framing (interim subsets vs the literal pre-push quartet) and an explicit irreducible manual gate for D3 layout/hover/export.Decisions surfaced for operator review (defaults chosen)
/admin/lineage/— tension with the issue's ideator-page link (ideators may be non-admin); alternative is an open/lineage/route.--forgejo-web-url/FORGEJO_WEB_URL, no fictional default.Plan only — operator reviews + approves before impl spawn.