[lokewate]

Summary

A task row kept a full copy of its render config, taken when the task started (task_records_v2.render_config). Every other template a task uses is read from the artifacts through the manifest version the task is pinned to, so the render config was the one template with a second copy. It is also the copy a future patch release could not reach: a patched form would no longer match a render config frozen on the row (its files, for one).

The row now keeps the id of the task template the task started from, and every reader resolves the template and its render config through the task's manifest version. Published artifacts never change, so a task still renders the config it started with.

It stores the task template id rather than the render config id because the task template is what the task started from: the render config is one thing read from it, and hosts need others (its meta for task names, its workflow). The row had no task template id before; ActiveTaskTemplateID names the active step's template and changes on every step.

#365 should merge first: it caches artifact files after their first load, so resolving the render config on every render does not fetch it from the bucket each time. The two do not depend on each other's code.

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation update
• Refactoring (no functional changes)
• Performance improvement
• Other (please describe):

Changes Made

• taskflow/renderconfig: new Load(ctx, set, record) returns a task's render config: the task template record.TaskTemplateID and the render config it names, both in manifest version record.ManifestVersion. A record with no task template id fails with ErrNoTaskTemplate. It is a function, not a TaskManager method, so hosts that hold a task record and the artifact set (file binding, read authz, review plugins) can call it.
• store.TaskRecord: RenderConfig (the JSON) is replaced by TaskTemplateID. The GORM model maps it to task_template_id. DeepCopy no longer has bytes to copy.
• StartTask records the task template's id. It still loads the render config, so a missing one fails the task at start rather than at its first render.
• GetTaskRenderInfo and ZoneViewAssembler.Assemble resolve the render config with renderconfig.Load. NewZoneViewAssembler takes the artifact set.
• Docs (architecture.md, integration-guide.md, template-reference.md, frontend-guide.md) describe the id on the record instead of the snapshot, and the migration the table needs.

Testing

• I have tested this change locally
• I have added tests that prove my fix is effective or that my feature works
• I have tested edge cases
• All existing tests pass

taskflow/renderconfig/renderconfig_test.go:

• The same record renders v1's or v2's render config by its manifest version, where v2's task template names a different render config.
• A record without a task template id, or without a manifest version, is refused.
• A task template, or a render config, the manifest does not list is not found.

The zone assembler, task manager and GORM model tests now build records with a task template id. The zone assembler refuses a record without one. go vet ./... and go test ./... pass.

Checklist

• My code follows the project's style guidelines
• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have checked that there are no merge conflicts

Related Issues

Related to OpenNSW/one-trade-artifacts#243

Additional Notes

Upgrading needs this migration on task_records_v2:

ALTER TABLE task_records_v2 ADD COLUMN IF NOT EXISTS task_template_id TEXT NOT NULL DEFAULT '';
ALTER TABLE task_records_v2 DROP COLUMN IF EXISTS render_config;

There is no backfill: rows written before this change have no task template id and cannot be rendered, so tasks started before the upgrade must be cleared. nsw-srilanka's switch to renderconfig.Load and its migration follow in a separate PR.

🤖 Generated with Claude Code

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 5bb0d546-1bf9-4337-adf0-ee249c0e35c7

Finishing Touches

📝 Generate docstrings

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[mushrafmim]

LGTM

[Aravinda-HWK]

LGTM